[PATCH] Replace CHECK_STATUS with torture macros

[Anoop C S]

On Tue, 2015-10-27 at 12:00 +0100, Michael Adam wrote:
> On 2015-10-27 at 11:54 +0530, Anoop C S wrote:
> > On Mon, 2015-10-26 at 11:26 +0100, Michael Adam wrote:
> > > Hi Anoop,
> > > 
> > > The patch looks clean now.
> > > But I have one more request:
> > > 
> > > Just as you split the addition of torture_asserts for
> > > the smb2_unlink calls (that had no assert with them
> > > before) into a patch of their own, could you do the
> > > same with the smb2_util_close calls?
> > > I just would like the patch that is called
> > > "Replace CHECK_STATUS with torture_assert macros"
> > > to do that and not more (adding checks where there
> > > were none before). You could do a third patch or
> > > add add these hunks to the patch with the unlink part.
> > > It is up to you.
> > > 
> > 
> > Attaching 3 patches as per Michael's comments.
> 
> Thanks!
> 
> This looks much cleaner now.
> 
> Just one more remark (sorry..):
> 

No problem.

> In the first patch (replacing CHECK_STATUS by torture asserts),
> when you are not checking for STATUS_OK, then your messages
> 'smb2_foobar failed' are misleading. In those few cases, you
> should rather utter a message like 'smb2_foobar gave unexpected
> result' or similar..
> 
> 
> Example:
> 
> > diff --git a/source4/torture/smb2/session.c
> > b/source4/torture/smb2/session.c
> > index 798230b..92f404c 100644
> > --- a/source4/torture/smb2/session.c
> > +++ b/source4/torture/smb2/session.c
> > @@ -105,7 +98,10 @@ bool test_session_reconnect1(struct
> > torture_context *tctx, struct smb2_tree *tre
> >  	qfinfo.generic.level = RAW_FILEINFO_POSITION_INFORMATION;
> >  	qfinfo.generic.in.file.handle = _h1;
> >  	status = smb2_getinfo_file(tree, mem_ctx, &qfinfo);
> > -	CHECK_STATUS(status, NT_STATUS_USER_SESSION_DELETED);
> > +	torture_assert_ntstatus_equal_goto(tctx, status,
> > +					  
> >  NT_STATUS_USER_SESSION_DELETED,
> > +					   ret, done,
> > +					   "smb2_getinfo_file
> > failed");
> 
> Here failure of smb2_getinfo_file is expected and succes
> of the function call would actually be a failure of the test
> just any status code other than NT_STATUS_USER_SESSION_DELETED
> would be.
> 

I have made necessary changes. Please let me know if I am missing some
parts.

> Cheers - Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s4.torture.smb2-session-Replace-CHECK_STATUS-with-to.patch
Type: text/x-patch
Size: 26166 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20151027/0ff46337/0001-s4.torture.smb2-session-Replace-CHECK_STATUS-with-to.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-s4.torture.smb2-session-Add-torture-assert-for-unlin.patch
Type: text/x-patch
Size: 5552 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20151027/0ff46337/0002-s4.torture.smb2-session-Add-torture-assert-for-unlin.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-s4.torture.smb2-session-Add-torture-assert-for-close.patch
Type: text/x-patch
Size: 1730 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20151027/0ff46337/0003-s4.torture.smb2-session-Add-torture-assert-for-close.bin>