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
[email protected]
https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers
