On Thu, 2014-05-08 at 07:18 +0200, Svante Signell wrote:
> On Thu, 2014-05-08 at 02:05 +0100, Steven Hartland wrote:
> > ----- Original Message -----
> > From: "Svante Signell" <svante.sign...@gmail.com>
> > To: "rrdtool dev list" <rrd-developers@lists.oetiker.ch>
> > Sent: Tuesday, April 29, 2014 1:35 PM
> > Subject: [rrd-developers] RFC: [PATCH] Portability by avoiding PATH_MAX
> >
> >
> > > Hello,
> > This looks like it would cause a lot of unnessary malloc free surely
> > the correct fix for this is to fix the OS config to define PATH_MAX.
> >
> > Quick look at the patches also show its quite broken:-
> >
> > + tmp = malloc(len);
> > + snprintf(tmp, len, "%s/%s", config_base_dir, *filename);
> > *filename = tmp;
> > + free(tmp);
> >
> > So you just malloced some memory, assigned the pointer to it then freed
> > the memory, so your now going to get a use after free and BOOM!
>
> This is of course wrong, thanks! You see the reason for having tests
> together with valgrind to find out these problems. Did you find more?
I even had test code for this function, see attachment. Unfortunately
the change did not make it into the patch. Sorry for missing to update
the patch.
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
#include <assert.h>
#include <ctype.h>
static char *config_base_dir = NULL;
static void get_abs_path(char **filename)
{
char *tmp = NULL;
int len = 0;
config_base_dir = "~/DEBs";
assert(filename != NULL && *filename != NULL);
if (config_base_dir == NULL || **filename == '/')
return;
len = strlen(config_base_dir) + 1 + strlen(*filename) + 1;
tmp = malloc(len);
snprintf(tmp, len, "%s/%s", config_base_dir, *filename);
*filename = tmp;
//NOK!! free(tmp);
} /* }}} static int get_abs_path */
int main (int argc, char **argv)
{
static char *journal_dir = NULL, *journal_dir_dup = NULL;
static char *base_realpath = NULL;
int option;
char *file = "~srs/DEBs";
//size_t len;
while ((option = getopt(argc, argv, "j:b:x")) != -1)
switch (option)
{
case 'j':
printf("optarg=%s\n",optarg);
journal_dir = realpath((const char *)optarg, NULL);
printf("case 'j': journal_dir=%s\n",journal_dir);
if (journal_dir)
journal_dir_dup = strdup(journal_dir);
free (journal_dir);
printf("after strdup: journal_dir_dup=%s\n",journal_dir_dup);
free (journal_dir_dup);
break;
case 'b':
/*
if (config_base_dir != NULL)
free (config_base_dir);
*/
printf("optarg=%s\n",optarg);
if (optarg != NULL)
config_base_dir = strdup (optarg);
if (config_base_dir == NULL)
{
fprintf (stderr, "read_options: strdup failed.\n");
return 1;
}
if ((base_realpath = realpath(config_base_dir, NULL)) == NULL)
{
fprintf (stderr, "Failed to canonicalize the base directory '%s': "
"%s\n", config_base_dir, strerror(errno));
free(config_base_dir);
return 1;
}
printf ("case 'b': base_realpath=%s\n", base_realpath);
free(config_base_dir);
free (base_realpath);
break;
case 'x':
get_abs_path (&file);
printf ("case 'x': file=%s\n",file);
free (file);
break;
case '?':
if (optopt == 'j' || optopt == 'b')
fprintf (stderr, "Option -%c requires an argument.\n", optopt);
else if (isprint (optopt))
fprintf (stderr, "Unknown option `-%c'.\n", optopt);
else
fprintf (stderr,
"Unknown option character `\\x%x'.\n",
optopt);
return 1;
default:
return 1;
}
return 0;
}
_______________________________________________
rrd-developers mailing list
rrd-developers@lists.oetiker.ch
https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers
