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
>
