On Mon, Jan 9, 2017 at 12:30 PM, Jeff King <peff@xxxxxxxx> wrote: > I also wonder if it is worth just using argv_array. We do not care about > NULL-terminating the list here, but it actually works pretty well as a > generic string-array class (and keeping a NULL at the end of any > array-of-pointers is a reasonable defensive measure). Then the function > becomes: > > argv_array_clear(&colors); > ... > if (!color_parse_mem(..., color)) > argv_array_push(&colors, color); > ... > argv_array_push(&colors, GIT_COLOR_RESET); > /* graph_set_column_colors takes a max-index, not a count */ > graph_set_column_colors(colors.argv, colors.argc - 1); > > It is not much shorter than ALLOC_GROW(), but IMHO it is easier to read. Indeed. My only complaint is it's "argv_array_" and not "string_array_" but we could think about renaming it later when we see another use like this. -- Duy