On Mon, 2013-08-19 at 18:33 +0200, Tobias Oetiker wrote: > > > 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 ...
Yes, there is a memory leak introduced by allocing the string instead of having a constant one. Regarding the API the only code calling get_path() is in rrd_client.c. > 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. What about the new version, here get_path is defined differently depending on whether POSIX.1-2008 is supported or not by #if-ing on _POSIX_VERSION (see man realpath). However, I can use a different name for the alloced version compared to the constant version. Thanks, Svante
#include <string.h> #include <stdio.h> #include <stdlib.h> #include <unistd.h> #include <errno.h> #include <pthread.h> #include <limits.h> static char *sd_path = "/tmp"; /* cache the path for sd */ static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; /* Undefines the _POSIX_VERSION */ #undef _POSIX_VERSION /* 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. */ #if (_POSIX_VERSION >= 200809L) static char *get_path (const char *path) /* {{{ */ { char *ret = (char *)path; 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; } } return (char *)path; } /* }}} char *get_path */ #else static const char *get_path (const char *path, char *resolved_path) /* {{{ */ { const char *ret = path; int is_unix = 0; if ((path == NULL) || (resolved_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, resolved_path); 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; } } return path; } /* }}} char *get_path */ #endif int rrdc_create (const char *filename) { #if (_POSIX_VERSION >= 200809L) char *file_path; #else char file_path[PATH_MAX]; #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); #if (_POSIX_VERSION >= 200809L) file_path = get_path (filename); printf("file_path=%s\n", file_path); if (file_path == NULL) #else filename = get_path (filename, file_path); printf("filename=%s\n", file_path); if (filename == NULL) #endif { pthread_mutex_unlock (&lock); return (-1); } pthread_mutex_unlock (&lock); /* Cannot be freed if using const */ #if (_POSIX_VERSION >= 200809L) free(file_path); #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