On Sat, Feb 11, 2023 at 4:11 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Masahiro Yamada <masahiroy@xxxxxxxxxx> 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? Sorry, I meant: "The same issue already exists for isprint()" #define isprint(x) ((x) >= 0x20 && (x) <= 0x7e) is not robust against pointer increment. > > > #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. > > At first, I hesitated to move these to git-compat-util.h for this reason, but I noticed isprint() was in the same situation. So, I decided to go. -- Best Regards Masahiro Yamada
