Karthik Nayak <karthik.188@xxxxxxxxx> writes: > Introduce color_atom_parser() which will parse a "color" atom and > store its color in the "used_atom" structure for further usage in > populate_value(). > > Helped-by: Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx> > Helped-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx> > Signed-off-by: Karthik Nayak <Karthik.188@xxxxxxxxx> > --- > ref-filter.c | 29 ++++++++++++++++++++--------- > 1 file changed, 20 insertions(+), 9 deletions(-) > > diff --git a/ref-filter.c b/ref-filter.c > index b54c872..9708d67 100644 > --- a/ref-filter.c > +++ b/ref-filter.c > @@ -29,6 +29,9 @@ typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type; > static struct used_atom { > const char *name; > cmp_type type; > + union { > + char *color; > + } u; > } *used_atom; > static int used_atom_cnt, need_tagged, need_symref; > static int need_color_reset_at_eol; > @@ -53,6 +56,18 @@ static int match_atom_name(const char *name, const char *atom_name, const char * > return 1; > } > > +static void color_atom_parser(struct used_atom *atom) > +{ > + if (!match_atom_name(atom->name, "color", (const char **)&atom->u.color)) > + die("BUG: parsing non-'color'"); > + if (!atom->u.color) > + die(_("expected format: %%(color:<color>)")); > + /* atom->u.color points to part of atom->name */ > + atom->u.color = xstrdup(atom->u.color); > + if (color_parse(atom->u.color, atom->u.color) < 0) > + die(_("invalid color value: %s"), atom->u.color); Is this calling color_parse() from color.c? The function wants the destination to be at least COLOR_MAXLEN, but I do not see where the piece memory pointed by atom->u.color is guaranteed to be that long in the new code. Looking at the code removed by this patch, it used to correctly use a buffer that is COLOR_MAXLEN bytes long. So... const char *color_value; if (!match_atom_name(atom->name, "color", color_value)) die("BUG: parsing non-'color'"); if (!color_value) die(_("expected format: %%(color:<color>)")); atom->u.color = xmalloc(COLOR_MAXLEN); if (color_parse(color_value, atom->u.color) < 0) die(_("invalid color value: %s"), color_value); or even define it in the union, i.e. union { char color[COLOR_MAXLEN]; } u; and then use atom->u.color[] in-place? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html