We are going to completely ignore diffs which change multiple idioms at once.
That is how mistakes get made. > On Sun, May 04, 2014 at 02:38:40AM -0400, Jean-Philippe Ouellet wrote: > > Hello, > > > > I've gone through lib/libssl/src/apps with the goal of making {m,c,re}alloc > > uses more idiomatic, adding error checking in some places where missing, > > and some minor style unification. > > > > Feedback appreciated, better patches to come after the semester ends. > > > > > > Index: apps.c > > =================================================================== > > RCS file: /cvs/src/lib/libssl/src/apps/apps.c,v > > retrieving revision 1.45 > > diff -u -p -r1.45 apps.c > > --- apps.c 3 May 2014 16:03:54 -0000 1.45 > > +++ apps.c 4 May 2014 06:07:45 -0000 > > @@ -209,13 +209,12 @@ chopup_args(ARGS * arg, char *buf, int * > > *argc = 0; > > *argv = NULL; > > > > - i = 0; > > if (arg->count == 0) { > > arg->count = 20; > > - arg->data = (char **)malloc(sizeof(char *) * arg->count); > > + arg->data = calloc(arg->count, sizeof(char *)); > > + if (arg->data == NULL) > > + return 0; > > } > > - for (i = 0; i < arg->count; i++) > > - arg->data[i] = NULL; > > > > num = 0; > > p = buf; > > @@ -232,8 +231,7 @@ chopup_args(ARGS * arg, char *buf, int * > > if (num >= arg->count) { > > char **tmp_p; > > int tlen = arg->count + 20; > > - tmp_p = (char **) realloc(arg->data, > > - sizeof(char *) * tlen); > > + tmp_p = reallocarray(arg->data, tlen, sizeof(char *)); > > if (tmp_p == NULL) > > return 0; > > arg->data = tmp_p; > > @@ -266,7 +264,7 @@ chopup_args(ARGS * arg, char *buf, int * > > } > > *argc = num; > > *argv = arg->data; > > - return (1); > > + return 1; > > } > > > > int > > @@ -404,7 +402,28 @@ password_callback(char *buf, int bufsiz, > > ok = UI_add_input_string(ui, prompt, ui_flags, buf, > > PW_MIN_LENGTH, bufsiz - 1); > > if (ok >= 0 && verify) { > > - buff = (char *) malloc(bufsiz); > > + buff = malloc(bufsiz); > > + /* > > + * NULL returns appear to be handled by the following: > > + * > > + * UI_add_verify_string(x, x, x, buff=NULL, x, x, x) -> > > + * general_allocate_string(x, x, x, UIT_VERIFY, x, \ > > + * result_buf=NULL, x, x, x) -> > > + * general_allocate_prompt(x, x, x, x, x, NULL) -> > > + * ret = NULL; > > + * if (type == UIT_VERIFY && result_buf == NULL) > > + * UIerr(...); and dont touch ret > > + * returns NULL > > + * returns -1 > > + * returns -1 > > + * > > + * So, we /should/ (maybe) be good. Not calling UIerr() > > + * could very well have some well-hidden side-effects > > + * that I would definitly not notice myself, so I'll > > + * leave this as is without an explicit check here. > > + * Maybe somebody who knows better than I has a better > > + * suggestion? > > + */ > > ok = UI_add_verify_string(ui, prompt, ui_flags, buff, > > PW_MIN_LENGTH, bufsiz - 1, buf); > > } > > @@ -1830,26 +1849,30 @@ parse_yesno(const char *str, int def) > > X509_NAME * > > parse_name(char *subject, long chtype, int multirdn) > > { > > - size_t buflen = strlen(subject) + 1; /* to copy the types and > > - * values into. due to > > - * escaping, the copy can > > - * only become shorter */ > > - char *buf = malloc(buflen); > > - size_t max_ne = buflen / 2 + 1; /* maximum number of name elements */ > > - char **ne_types = malloc(max_ne * sizeof(char *)); > > - char **ne_values = malloc(max_ne * sizeof(char *)); > > - int *mval = malloc(max_ne * sizeof(int)); > > + size_t buflen, max_ne; > > + char **ne_types, **ne_values, *buf, *sp, *bp; > > + int *mval, i, nid, ne_num = 0; > > + X509_NAME *n = NULL; > > > > - char *sp = subject, *bp = buf; > > - int i, ne_num = 0; > > + /* Due to escaping, buf can never be bigger than subject. */ > > + buflen = strlen(subject + 1); > ^^^^ > In addition to already mentioned mistake in above strlen() > > > - X509_NAME *n = NULL; > > - int nid; > > + /* maximum number of name elements */ > > + max_ne = buflen / 2 + 1; > > + > > + buf = malloc(buflen); > > + ne_types = malloc(max_ne); > > + ne_values = malloc(max_ne); > > above two malloc() calls are inconsistent with original > code, which passed 'max_ne * sizeof(char *)' to malloc(). > > --patrick >