[PATCH] Fix smbtorture memory leaks

[Andreas Schneider via samba-technical]

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

[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));\
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]

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]

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

[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
feel free to push both commits with:
Reviewed-by: David Disseldorp <***@samba.org>

Cheers, David

[Andreas Schneider via samba-technical]

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