Hi Svante, Today Svante Signell wrote:
> On Fri, 2013-08-16 at 14:22 +0200, Tobias Oetiker wrote: > > Hi Svante, > > > > > > > OK, I will continue next with rrd_client.c. Is that one built into the > > > library librrd*.so* ? > > > > yes, but it can also be used standalone > > How? in the sense that it has little dependence on rrdtool and the license is chosen to allow linking non gnu projects > > > One issue with rrd_client:get_path is the const definitions. In order to > > > free the dynamically allocated strings, to avoid memory leaks const has > > > to be abandoned in some places. OK? > > > > I can only tell you when I see the patch ... if I am reading your code corectly, you are causing a memory leak by makeing get_path return an allocated string instead of a constant ... the root cause of the problem is that you are changeing the API of get_path ... for backward compatibility this is rather sub optimal ... better choose a new function name and whoever is using it knows that they have to free the data they get from the call. cheers tobi > Attached below is a stripped down example from rrd_client.c. Defining > NOT_CONST in the code and > gcc -g -Wall test_get_path.c > valgrind --leak-check=full ./a.out . > gives: > > valgrind --leak-check=full ./a.out . > ==5155== Memcheck, a memory error detector > ==5155== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al. > ==5155== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright > info > ==5155== Command: ./a.out . > ==5155== > calling rrd_create > rrdc_create: filename=. > filename=. > get_path: path=. > absfilename=/my/path/to/rrdtool/rrdtool-git > rc = 0 (0=OK) > ==5155== > ==5155== HEAP SUMMARY: > ==5155== in use at exit: 0 bytes in 0 blocks > ==5155== total heap usage: 1 allocs, 1 frees, 4,096 bytes allocated > ==5155== > ==5155== All heap blocks were freed -- no leaks are possible > ==5155== > ==5155== For counts of detected and suppressed errors, rerun with: -v > ==5155== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 4 from 4) > > However uncommenting #undef NOT_CONST results in a small memory leak: > > ==5190== > ==5190== HEAP SUMMARY: > ==5190== in use at exit: 4,096 bytes in 1 blocks > ==5190== total heap usage: 1 allocs, 0 frees, 4,096 bytes allocated > ==5190== > ==5190== 4,096 bytes in 1 blocks are definitely lost in loss record 1 of > 1 > ==5190== at 0x4C28BED: malloc > (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > ==5190== by 0x4E6F5F7: realpath@@GLIBC_2.3 (canonicalize.c:79) > ==5190== by 0x400842: get_path (test_get_path.c:46) > ==5190== by 0x400905: rrdc_create (test_get_path.c:83) > ==5190== by 0x400983: rrd_create (test_get_path.c:107) > ==5190== by 0x4009F7: main (test_get_path.c:124) > ==5190== > ==5190== LEAK SUMMARY: > ==5190== definitely lost: 4,096 bytes in 1 blocks > ==5190== indirectly lost: 0 bytes in 0 blocks > ==5190== possibly lost: 0 bytes in 0 blocks > ==5190== still reachable: 0 bytes in 0 blocks > ==5190== suppressed: 0 bytes in 0 blocks > ==5190== > ==5190== For counts of detected and suppressed errors, rerun with: -v > ==5190== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 4 from 4) > > Heap usage it OK in both cases. What do you think, should the > memory-leak-free version be used for patching rrd_client.c (I have > problems to test this directly, see above) > > -- Tobi Oetiker, OETIKER+PARTNER AG, Aarweg 15 CH-4600 Olten, Switzerland http://it.oetiker.ch [email protected] ++41 62 775 9902 / sb: -9900 _______________________________________________ rrd-developers mailing list [email protected] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers
