[Patches] avoid segfault in durable-open test
obnox at samba.org
Thu Jun 25 07:41:35 MDT 2015
On 2015-06-25 at 10:22 +1200, Douglas Bagnall wrote:
> hi Michael,
> Amended patches attached.
> On 19/06/15 22:10, Michael Adam wrote:
> > 1. in the first patch, can you do if (tree1 != NULL) instead of
> > if (tree1) ?
> I've taken the liberty of adding your "reviewed by" on this one
That's fine by me.
So with a second team reviewer, we could push this.
> > 2. For the second patch: we try to get rid of these locally
> > defined CHECK_FOO macros.
> > There are macros in lib/torture/torture.h of the form
> > torture_assert
> > torture_assert_foobar
> > torture_assert_foobar_goto
> > I sugget using torture_assert_ntstatus_equal_goto().
> > This is more writing than CHECK...(), but it is the
> > standard way. Could you do that instead?
> ...but not this one.
Thanks for doing the conversion.
This seems the right approach, but some toughts:
- if checking for OK you can use the simpler
- I quite frankly think it is not much use
to hand in messages like "Status should be OK"
or "Status should be FOO". The macro will already
print something like this.
My idea would be to have the message as something like
"smb2_create failed" or "expected smb2_create to fail"
depending on what call that status actually came from.
This would add extra value!
Would you take another round on this one?
> > Thanks - Michael
> > On 2015-06-19 at 16:18 +1200, Douglas Bagnall wrote:
> >> hi all,
> >> After my last patches, an autobuild on sn-devel segfaulted in the
> >> samba3.smb2.durable-open.open2-lease(nt4_dc) test. The first of these
> >> patches turns the segfault into a normal failure (or at least it would
> >> if we could reproduce it). The second should make the branching in these
> >> tests a little bit clearer for the next novice to wander this way.
> >> I touched that file but I can't see how its related to the error.
> >> Douglas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 198 bytes
Desc: not available
More information about the samba-technical