On Wed, Oct 2, 2024 at 6:56 AM Patrick Steinhardt <ps@xxxxxx> wrote: > Handle allocation failures in `parse_names()` by returning `NULL` in > case any allocation fails. While at it, refactor the function to return > the array directly instead of assigning it to an out-pointer. > > Signed-off-by: Patrick Steinhardt <ps@xxxxxx> > --- > diff --git a/reftable/basics.c b/reftable/basics.c > @@ -152,14 +152,26 @@ void parse_names(char *buf, int size, char ***namesp) > REFTABLE_ALLOC_GROW(names, names_len + 1, names_cap); > - names[names_len++] = xstrdup(p); > + if (!names) > + goto err; Am I reading this correctly? Presumably, `names_len` can be non-zero here, right? And we now check for names==NULL to detect an allocation failure... > + names[names_len] = reftable_strdup(p); > + if (!names[names_len++]) > + goto err; > } > p = next + 1; > } > > REFTABLE_REALLOC_ARRAY(names, names_len + 1); > names[names_len] = NULL; > - *namesp = names; > + > + return names; > + > +err: > + for (size_t i = 0; i < names_len; i++) > + reftable_free(names[i]); ... and then we potentially index into names[] because `names_len` is non-zero, thus crash because `names` is NULL. > + reftable_free(names); > + return NULL; > }
