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

[Junio C Hamano <gitster@pobox.com>]

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.