+1 , this makes more sense than trying to assume the lifetime of passed pointers.
Though I usually try writing a string_set helper to avoid too much boilerplate when there are lots of parameters. On Fri, Aug 15, 2014 at 12:06 PM, Ted Unangst <[email protected]> wrote: > This diff is incomplete, but shows the direction I'm headed. > > The current config struct keeps a pointer to various strings (ca file, > etc.). This causes (or will cause) two kinds of trouble. > > 1. error prone. it's reasonable to create a ca file path by > snprintf into a stack buffer. when that buffer disappears -> boom. > > 2. unworkable. the fix for that, having the caller strdup, is > unworkable because there's no way to get the string back later to free > it. > > Instead, ressl should copy all parameters as necessary and > free them. This does introduce an error case into formerly void > functions, but I think that's ok. The alternative would be to use > fixed arrays inside ressl_config, but that seems much worse. > > Index: ressl_config.c > =================================================================== > RCS file: /cvs/src/lib/libressl/ressl_config.c,v > retrieving revision 1.7 > diff -u -p -r1.7 ressl_config.c > --- ressl_config.c 6 Aug 2014 01:54:01 -0000 1.7 > +++ ressl_config.c 15 Aug 2014 16:58:50 -0000 > @@ -48,13 +48,20 @@ ressl_config_new(void) > void > ressl_config_free(struct ressl_config *config) > { > + free(config->ca_file); > free(config); > } > > -void > +int > ressl_config_set_ca_file(struct ressl_config *config, char *ca_file) > { > - config->ca_file = ca_file; > + free(config->ca_file); > + if (ca_file != NULL) { > + if ((config->ca_file = strdup(ca_file)) == NULL) > + return -1; > + } else > + config->ca_file = NULL; > + return 0; > } > > void > >
