Re: master 64e25cd: More robust NS hex colour string parsing

[Pip Cet]

On Mon, Jun 8, 2020 at 12:07 PM Mattias Engdegård
<mattiase@savannah.gnu.org> wrote:
> branch: master
> commit 64e25cde324b2e270acf82958abb59018e67f841
> Author: Mattias Engdegård <mattiase@acm.org>
> Commit: Mattias Engdegård <mattiase@acm.org>
>
>     More robust NS hex colour string parsing
>
>     Invalid arguments to color-values, such as "#abcdefg" or "#1234", or
>     valid ones like "#111222333", should not yield nonsense values.
>
>     * src/nsterm.m (ns_get_color):
>     Only accept "#RGB" strings with 1-4 digits per components, equal number
>     of digits each, and no trailing characters.  Parse 12-bit colours
>     correctly.
> ---
>  src/nsterm.m | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/src/nsterm.m b/src/nsterm.m
> index 1953138..3dc7e1d 100644
> --- a/src/nsterm.m
> +++ b/src/nsterm.m
> @@ -2399,20 +2399,23 @@ ns_get_color (const char *name, NSColor **col)
>      scaling = (snprintf (hex, sizeof hex, "%s", name + 4) - 2) / 3;
>    else if (name[0] == '#')        /* An old X11 format; convert to newer */
>      {
> -      int len = (strlen(name) - 1);
> -      int start = (len % 3 == 0) ? 1 : len / 4 + 1;
> -      int i;
> -      scaling = strlen(name+start) / 3;
> -      for (i = 0; i < 3; i++)
> -       sprintf (hex + i * (scaling + 1), "%.*s/", scaling,
> -                name + start + i * scaling);
> -      hex[3 * (scaling + 1) - 1] = '\0';
> +      int len = 0;
> +      while (isxdigit (name[len + 1]))
> +        len++;
> +      if (name[len + 1] == '\0' && len >= 1 && len <= 12 && len % 3 == 0)
> +        {
> +          scaling = len / 3;
> +          for (int i = 0; i < 3; i++)
> +            sprintf (hex + i * (scaling + 1), "%.*s/", scaling,
> +                     name + 1 + i * scaling);
> +          hex[3 * (scaling + 1) - 1] = '\0';
> +        }
>      }

I believe there's very similar code for the X case, where we also
translate #fff to rgb:f/f/f. That code has annoyed me lately by
producing nonsensical GCC warnings, maybe you'd like to replace it
with yours? Having two different versions of this seems superfluous
(I'm not doubting the correctness of your version, which I guess means
I'm doubting that of mine).