Re: [PATCH v2 1/3] dtc: Refactor character literal parsing code

[Anton Staaf <robotboy-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>]

On Wed, Sep 7, 2011 at 8:22 PM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Wed, Sep 07, 2011 at 04:15:38PM -0700, Anton Staaf wrote:
>> Move the parsing of hex, octal and escaped characters from data.c
>> to util.c where it can be used for character literal parsing within
>> strings as well as for stand alone C style character literals.
>
> [snip]
>> +char get_oct_char(const char *s, int *i)
>
> Probably make these static, since I can't really see a use for them
> other than in the escape processing code.

Yup, will do.

>> +{
>> +     char x[4];
>> +     char *endx;
>> +     long val;
>> +
>> +     x[3] = '\0';
>> +     strncpy(x, s + *i, 3);
>> +
>> +     val = strtol(x, &endx, 8);
>> +
>> +     assert(endx > x);
>> +
>> +     (*i) += endx - x;
>> +     return val;
>> +}
>> +
>> +char get_hex_char(const char *s, int *i)
>> +{
>> +     char x[3];
>> +     char *endx;
>> +     long val;
>> +
>> +     x[2] = '\0';
>> +     strncpy(x, s + *i, 2);
>> +
>> +     val = strtol(x, &endx, 16);
>> +     if (!(endx  > x))
>> +             die("\\x used with no following hex digits\n");
>> +
>> +     (*i) += endx - x;
>> +     return val;
>> +}
>> +
>> +char get_escape_char(const char *s, int *i)
>
> Yes, splitting this function off is quite nice.
>
> [snip]
>> +char get_escape_char_exact(const char *s, int len)
>> +{
>> +     int     j = 1; //skip intial "\"
>> +     char    c = get_escape_char(s, &j);
>> +
>> +     if (j != len)
>> +             die("Extra characters at end of character literal '%s' "
>> +                 "(%d != %d)\n", s, j, len);
>> +
>> +     return c;
>> +}
>
> This I'm not a fan of.  Handling erroneous input with a die() in a
> low-level helper like this is pretty nasty (I know there are other
> instances of it).  I'd prefer to see the action handling the error in
> the caller though, where it's better equipped to know a reasonable way
> of informing the user.

Can do, this behavior was copied from another _exact function, I'll
switch to doing the check in the parser rule.

Thanks,
    Anton

> Some more on error handling in comments on your next patch.
>
> --
> David Gibson                    | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
>                                | _way_ _around_!
> http://www.ozlabs.org/~dgibson
>