[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