Various fixes missing

[sevan]

Hi,
There are various issues with bstrlib which are highlighted using tools such as Coverity Scan some of which have been fixed by @msteinert in his fork of bstrlib could you look to integrating Coverity into your workflow & bringing in the fixes already made in the fork?

[websnarf]

This fork is from a much earlier version of Bstrlib. All the issues he addressed have already been fixed in the latest version of Bstrlib or have been incorrectly changed in his code. For example, bSecureDestroy must not attempt to free a bstring if it is constant, write-protected, or obviously corrupted.

[msteinert]

I dropped the updated code into my test harness and ran it under Valgrind. It appears that the memory leak I reported is still there:

Running suite(s): bstr-aux
100%: Checks: 14, Failures: 0, Errors: 0
==8428== 2,096 (32 direct, 2,064 indirect) bytes in 1 blocks are definitely lost in loss record 5 of 6
==8428==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8428==    by 0x534D138: bsopen (bstrlib.c:2053)
==8428==    by 0x5348F42: bsFromBstrRef (bstraux.c:240)
==8428==    by 0x5348F42: bUuDecodeEx (bstraux.c:597)
==8428==    by 0x402BB8: core_010 (testaux.c:378)
==8428==    by 0x406DD1: tcase_run_tfun_nofork.isra.11 (in /opt/home/mike/src/git/bstring/tests/.libs/lt-testaux)
==8428==    by 0x40703D: srunner_run (in /opt/home/mike/src/git/bstring/tests/.libs/lt-testaux)
==8428==    by 0x402420: main (testaux.c:506)
==8428== 
==8428== 2,096 (32 direct, 2,064 indirect) bytes in 1 blocks are definitely lost in loss record 6 of 6
==8428==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8428==    by 0x534D138: bsopen (bstrlib.c:2053)
==8428==    by 0x5348EC8: bsUuDecode (bstraux.c:710)
==8428==    by 0x5348F55: bUuDecodeEx (bstraux.c:599)
==8428==    by 0x402BB8: core_010 (testaux.c:378)
==8428==    by 0x406DD1: tcase_run_tfun_nofork.isra.11 (in /opt/home/mike/src/git/bstring/tests/.libs/lt-testaux)
==8428==    by 0x40703D: srunner_run (in /opt/home/mike/src/git/bstring/tests/.libs/lt-testaux)
==8428==    by 0x402420: main (testaux.c:506)
==8428== 
FAIL: tests/testaux

I don't recall why I changed the bSecureDestroy macro. Perhaps I was confused about its purpose. I vaguely recall thinking that the original implementation could lead to a memory leak in user code.

Anyway, thanks for making this code available. I mainly forked so I could play around with Travis-CI, Coverity Scan, and various other tooling. No disrespect intended!

[websnarf]

I somehow lost track of this fix. Thanks, Mike. No disrespect was inferred -- forking is part of the open source world, and obviously you fixed real bugs that existed at some point. I have incorporated your change in the latest update.

[sevan]

Great work guys :)
@websnarf Would you consider giving Coverity a spin to see what they're reporting is of concern?