[Patches] avoid segfault in durable-open test

Michael Adam obnox at samba.org
Fri Jun 19 04:10:43 MDT 2015


Hi Douglas,

thanks for your patches!

Some comments:

1. in the first patch, can you do if (tree1 != NULL) instead of
   if (tree1) ?

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?

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

> >From 1e2b7eff6eefa0a412603ef575d7d9955752e0c1 Mon Sep 17 00:00:00 2001
> From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
> Date: Fri, 19 Jun 2015 15:20:26 +1200
> Subject: [PATCH 1/2] Avoid segfault in durable_open tests
> 
> There are "goto done"s hiding in CHECK_STATUS in parts of the code
> where tree1 is unequivocally NULL.
> 
> Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
> ---
>  source4/torture/smb2/durable_open.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/source4/torture/smb2/durable_open.c b/source4/torture/smb2/durable_open.c
> index fd6af33..7bb8e75 100644
> --- a/source4/torture/smb2/durable_open.c
> +++ b/source4/torture/smb2/durable_open.c
> @@ -2136,12 +2136,14 @@ static bool test_durable_open_open2_lease(struct torture_context *tctx,
>  	h1 = io1.out.file.handle;
>  
>   done:
> +	if (tree1){
> +		smb2_util_close(tree1, h1);
> +		smb2_util_unlink(tree1, fname);
> +		talloc_free(tree1);
> +	}
> +
>  	smb2_util_close(tree2, h2);
>  	smb2_util_unlink(tree2, fname);
> -	smb2_util_close(tree1, h1);
> -	smb2_util_unlink(tree1, fname);
> -
> -	talloc_free(tree1);
>  	talloc_free(tree2);
>  
>  	return ret;
> -- 
> 1.9.1
> 
> 
> >From 62c81c570ed8ad0de3ea78a6d2439342159c85e0 Mon Sep 17 00:00:00 2001
> From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
> Date: Fri, 19 Jun 2015 15:25:02 +1200
> Subject: [PATCH 2/2] rename CHECK_STATUS in durable_open.c
> 
> CHECK_STATUS() hides a `goto done` in its heart. If we rename it to
> CHECK_STATUS_OR_GOTO_DONE() the secret is out.
> 
> Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
> ---
>  source4/torture/smb2/durable_open.c | 162 ++++++++++++++++++------------------
>  1 file changed, 81 insertions(+), 81 deletions(-)
> 
> diff --git a/source4/torture/smb2/durable_open.c b/source4/torture/smb2/durable_open.c
> index 7bb8e75..33ac942 100644
> --- a/source4/torture/smb2/durable_open.c
> +++ b/source4/torture/smb2/durable_open.c
> @@ -49,7 +49,7 @@
>  		ret = false; \
>  	}} while (0)
>  
> -#define CHECK_STATUS(status, correct) do { \
> +#define CHECK_STATUS_OR_GOTO_DONE(status, correct) do { \
>  	if (!NT_STATUS_EQUAL(status, correct)) { \
>  		torture_result(tctx, TORTURE_FAIL, __location__": Incorrect status %s - should be %s", \
>  		       nt_errstr(status), nt_errstr(correct)); \
> @@ -153,7 +153,7 @@ static bool test_one_durable_open_open_oplock(struct torture_context *tctx,
>  	io.in.durable_open = true;
>  
>  	status = smb2_create(tree, mem_ctx, &io);
> -	CHECK_STATUS(status, NT_STATUS_OK);
> +	CHECK_STATUS_OR_GOTO_DONE(status, NT_STATUS_OK);
>  	_h = io.out.file.handle;
>  	h = &_h;
>  	CHECK_CREATED(&io, CREATED, FILE_ATTRIBUTE_ARCHIVE);
> @@ -298,7 +298,7 @@ static bool test_one_durable_open_open_lease(struct torture_context *tctx,
>  	io.in.durable_open = true;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150619/39a268c6/attachment.pgp>


More information about the samba-technical mailing list