[PATCH] Replace CHECK_STATUS with torture macros

Michael Adam obnox at samba.org
Tue Oct 27 11:00:37 UTC 2015


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..):

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.

Cheers - Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20151027/a2ff74c3/signature.sig>


More information about the samba-technical mailing list