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?
> > 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 ...
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)
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
#include <pthread.h>
static char *sd_path = "/tmp"; /* cache the path for sd */
static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
/* Defines if the returned path is const or not */
#define NOT_CONST 1
#undef NOT_CONST
/* get_path: Return a path name appropriate to be sent to the daemon.
*
* When talking to a local daemon (thru a UNIX socket), relative path names
* are resolved to absolute path names to allow for transparent integration
* into existing solutions (as requested by Tobi). Else, absolute path names
* are not allowed, since path name translation is done by the server.
*
* One must hold `lock' when calling this function. */
#ifdef NOT_CONST
static char *get_path (const char *path) /* {{{ */
#else
static const char *get_path (const char *path) /* {{{ */
#endif
{
#ifdef NOT_CONST
char *ret = (char *)path;
#else
const char *ret = path;
#endif
int is_unix = 0;
printf("get_path: path=%s\n", path);
if ((path == NULL) || (sd_path == NULL))
return (NULL);
if ((*sd_path == '/')
|| (strncmp ("unix:", sd_path, strlen ("unix:")) == 0))
is_unix = 1;
if (is_unix)
{
ret = realpath(path, NULL);
if (ret == NULL)
printf("realpath(%s): %s\n", path, strerror(errno));
return ret;
}
else
{
if (*path == '/') /* not absolute path */
{
printf("absolute path names not allowed when talking "
"to a remote daemon\n");
return NULL;
}
}
#ifdef NOT_CONST
return (char *)path;
#else
return path;
#endif
} /* }}} char *get_path */
int rrdc_create (const char *filename)
{
#ifdef NOT_CONST
char *absfilename;
#else
const char *absfilename;
#endif
if (filename == NULL) {
printf("rrdc_create: no filename specified\n");
return (-1);
} else
printf("rrdc_create: filename=%s\n", filename);
pthread_mutex_lock (&lock);
printf("filename=%s\n", filename);
absfilename = get_path (filename);
printf("absfilename=%s\n", absfilename);
if (absfilename == NULL)
{
pthread_mutex_unlock (&lock);
return (-1);
}
pthread_mutex_unlock (&lock);
/* Cannot be freed if using const */
#ifdef NOT_CONST
free(absfilename);
#endif
return(0);
}
int rrd_create(
int argc,
char **argv)
{
int rc;
optind = 0;
// option handling
rc = rrdc_create (argv[optind]);
printf("rc = %d (0=OK)\n",rc);
return rc;
}
int main(
int argc,
char *argv[])
{
if (argc != 2)
{
printf("error: option argument missing\n");
return(1);
}
if (argv[1])
{
printf("calling rrd_create\n");
rrd_create(argc - 1, &argv[1]);
}
else
{
printf("error: return\n");
return(1);
}
return (0);
}
_______________________________________________
rrd-developers mailing list
rrd-developers@lists.oetiker.ch
https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers
