Re: [PATCH 1/5] git-compat-util: add isblank() and isgraph()

>
> Masahiro Yamada <masahiroy@kernel.org> writes:
>
> > Use them with care because they are not robust against the pointer
> > increment, like isblank(*s++).
> >
> > The same issue already exists for isspace().
>
> Does it?

>
> >  #define sane_istest(x,mask) ((sane_ctype[(unsigned char)(x)] & (mask)) != 0)
> >  #define isascii(x) (((x) & ~0x7f) == 0)
> >  #define isspace(x) sane_istest(x,GIT_SPACE)
>
> sane_istest() evaluates x and mask only once, and isspace evaluates
> x only once, no?
>
>         isspace(*x++)
>         ->
>         sane_istest(*x++,GIT_SPACE)
>         ->
>         ((sane_ctype[(unsigned char)(*x++)] & GIT_SPACE) != 0)
>
> > +#define isblank(x) (isspace(x) || (x) == '\t')
> > +#define isgraph(x) (isprint(x) && !isspace(x))
>
> Given that all the other tests are implemented with just picking an
> integer from the table and checking bit(s), the above two look
> somewhat out of place.  The fact, as you noted, that they cannot be
> used safely does not help, either.
>
>