Discussion:
[PATCH] Fix smbtorture memory leaks
(too old to reply)
Andreas Schneider via samba-technical
2018-05-16 10:14:11 UTC
Permalink
Hi,

attached are two patches to address memory leaks. Those have been found by RH
coverity scans.


Please review and push if OK.

Thanks,


Andreas
--
Andreas Schneider GPG-ID: CC014E3D
Samba Team ***@samba.org
www.samba.org
David Disseldorp via samba-technical
2018-05-16 11:36:39 UTC
Permalink
- torture_assert_int_equal(tctx, smbc_get ##option(ctx), val, "failed " #option);
+ torture_assert_int_equal_goto(tctx, smbc_get ##option(ctx), val, ok, done, "failed " #option);
#define TEST_OPTION_STRING(option, val) \
torture_comment(tctx, "Testing smbc_set" #option "\n");\
smbc_set ##option(ctx, strdup(val));\
torture_comment(tctx, "Testing smbc_get" #option "\n");\
- torture_assert_str_equal(tctx, smbc_get ##option(ctx), val, "failed " #option);
+ torture_assert_str_equal_goto(tctx, smbc_get ##option(ctx), val, ok, done, "failed " #option);
Gah, I know this is just test code, but please don't add extra macros
that use magic labels and variables. My preference would be to just
move the torture_assert out of the TEST_OPTION_X() macros and into the
caller.

Cheers, David
David Disseldorp via samba-technical
2018-05-16 12:29:19 UTC
Permalink
Post by David Disseldorp via samba-technical
- torture_assert_int_equal(tctx, smbc_get ##option(ctx), val, "failed " #option);
+ torture_assert_int_equal_goto(tctx, smbc_get ##option(ctx), val, ok, done, "failed " #option);
#define TEST_OPTION_STRING(option, val) \
torture_comment(tctx, "Testing smbc_set" #option "\n");\
smbc_set ##option(ctx, strdup(val));\
The strdup() leak here needs to be fixed too. The comment above
justifying the strdup() is wrong as of
2d41b1ab78639abe4ae030ff482573f464564dd7.
Post by David Disseldorp via samba-technical
torture_comment(tctx, "Testing smbc_get" #option "\n");\
- torture_assert_str_equal(tctx, smbc_get ##option(ctx), val, "failed " #option);
+ torture_assert_str_equal_goto(tctx, smbc_get ##option(ctx), val, ok, done, "failed " #option);
Gah, I know this is just test code, but please don't add extra macros
that use magic labels and variables. My preference would be to just
move the torture_assert out of the TEST_OPTION_X() macros and into the
caller.
Hmm, the macros also magically use "tctx" and "ctx". I'd just drop
them altogether.

Cheers, David
Andreas Schneider via samba-technical
2018-05-16 12:54:16 UTC
Permalink
On Wednesday, 16 May 2018 14:29:19 CEST David Disseldorp via samba-technical
Post by David Disseldorp via samba-technical
Post by David Disseldorp via samba-technical
- torture_assert_int_equal(tctx, smbc_get ##option(ctx), val, "failed
"
Post by David Disseldorp via samba-technical
Post by David Disseldorp via samba-technical
#option); + torture_assert_int_equal_goto(tctx, smbc_get
##option(ctx),
Post by David Disseldorp via samba-technical
Post by David Disseldorp via samba-technical
val, ok, done, "failed " #option);> >
#define TEST_OPTION_STRING(option, val) \
torture_comment(tctx, "Testing smbc_set" #option "\n");\
smbc_set ##option(ctx, strdup(val));\
The strdup() leak here needs to be fixed too. The comment above
justifying the strdup() is wrong as of
2d41b1ab78639abe4ae030ff482573f464564dd7.
Post by David Disseldorp via samba-technical
torture_comment(tctx, "Testing smbc_get" #option "\n");\
- torture_assert_str_equal(tctx, smbc_get ##option(ctx), val, "failed
"
Post by David Disseldorp via samba-technical
Post by David Disseldorp via samba-technical
#option); + torture_assert_str_equal_goto(tctx, smbc_get
##option(ctx),
Post by David Disseldorp via samba-technical
Post by David Disseldorp via samba-technical
val, ok, done, "failed " #option);>
Gah, I know this is just test code, but please don't add extra macros
that use magic labels and variables. My preference would be to just
move the torture_assert out of the TEST_OPTION_X() macros and into the
caller.
Hmm, the macros also magically use "tctx" and "ctx". I'd just drop
them altogether.
See attached updated patchset which fixes libsmbclient and the test.


Please take a look again.

Thanks,


Andreas
--
Andreas Schneider GPG-ID: CC014E3D
Samba Team ***@samba.org
www.samba.org
David Disseldorp via samba-technical
2018-05-16 14:07:13 UTC
Permalink
Post by Andreas Schneider via samba-technical
See attached updated patchset which fixes libsmbclient and the test.
Please take a look again.
Thanks for the rework. Please squash in the minor fixes attached and
feel free to push both commits with:
Reviewed-by: David Disseldorp <***@samba.org>

Cheers, David
Andreas Schneider via samba-technical
2018-05-16 17:15:46 UTC
Permalink
On Wednesday, 16 May 2018 16:07:13 CEST David Disseldorp via samba-technical
Post by David Disseldorp via samba-technical
Post by Andreas Schneider via samba-technical
See attached updated patchset which fixes libsmbclient and the test.
Please take a look again.
Thanks for the rework. Please squash in the minor fixes attached and
Thanks, I've pushed the following patchset to master.


Andreas
--
Andreas Schneider GPG-ID: CC014E3D
Samba Team ***@samba.org
www.samba.org
Loading...