On Tue, Sep 15, 2015 at 12:07 PM, Jeff King <[email protected]> wrote:
> Our color parsing is designed to never exceed COLOR_MAXLEN
> bytes. But the relationship between that hand-computed
> number and the parsing code is not at all obvious, and we
> merely hope that it has been computed correctly for all
> cases.
>
> Let's mark the expected "end" pointer for the destination
> buffer and make sure that we do not exceed it.
>
> Signed-off-by: Jeff King <[email protected]>
> ---
> @@ -224,12 +227,18 @@ int color_parse_mem(const char *value, int value_len,
> char *dst)
> goto bad;
> }
>
> +#define OUT(x) do { \
> + if (dst == end) \
> + die("BUG: color parsing ran out of space"); \
> + *dst++ = (x); \
> +} while(0)
Hmm, can we have an #undef OUT before the #define OUT(...), or choose
a less conflict-likely name? In particular, I'm thinking about
preprocessor namespace pollution arising from sources out of our
control, such as was the case with 414382f (ewah/bitmap: silence
warning about MASK macro redefinition, 2015-06-03).
> +
> if (attr || !color_empty(&fg) || !color_empty(&bg)) {
> int sep = 0;
> int i;
>
> - *dst++ = '\033';
> - *dst++ = '[';
> + OUT('\033');
> + OUT('[');
>
> for (i = 0; attr; i++) {
> unsigned bit = (1 << i);
> @@ -237,24 +246,24 @@ int color_parse_mem(const char *value, int value_len,
> char *dst)
> continue;
> attr &= ~bit;
> if (sep++)
> - *dst++ = ';';
> - dst += sprintf(dst, "%d", i);
> + OUT(';');
> + dst += xsnprintf(dst, end - dst, "%d", i);
> }
> if (!color_empty(&fg)) {
> if (sep++)
> - *dst++ = ';';
> + OUT(';');
> /* foreground colors are all in the 3x range */
> - dst = color_output(dst, &fg, '3');
> + dst = color_output(dst, end - dst, &fg, '3');
> }
> if (!color_empty(&bg)) {
> if (sep++)
> - *dst++ = ';';
> + OUT(';');
> /* background colors are all in the 4x range */
> - dst = color_output(dst, &bg, '4');
> + dst = color_output(dst, end - dst, &bg, '4');
> }
> - *dst++ = 'm';
> + OUT('m');
> }
> - *dst = 0;
> + OUT(0);
> return 0;
> bad:
> return error(_("invalid color value: %.*s"), value_len, value);
> --
> 2.6.0.rc2.408.ga2926b9
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
