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