On 09/15/18 14:16, Todd C. Miller wrote: > On Sat, 15 Sep 2018 12:42:22 +0200, Martijn van Duren wrote: > >> While here, should we also remove any in favour of strchr? Only >> difference seems to be the return type (bool vs pointer). > > Note that any(NULL, ch) is safe whereas strchr(NULL, ch) will crash. > It is hard to say whether or not there are actual calls to any() > with a NULL string (most use a constant string) but this needs to > be checked before committing. > > - todd > It should be safe. There are 5 instances where any() isn't called with a string literal: Index: dol.c =================================================================== RCS file: /cvs/src/bin/csh/dol.c,v retrieving revision 1.21 diff -u -p -r1.21 dol.c --- dol.c 16 Dec 2017 10:27:21 -0000 1.21 +++ dol.c 15 Sep 2018 10:41:24 -0000 @@ -918,7 +918,7 @@ heredoc(Char *term) * If any ` in line do command substitution */ mbp = mbuf; - if (any(short2str(mbp), '`')) { + if (strchr(short2str(mbp), '`')) { /* * 1 arg to dobackp causes substitution to be literal. Words are * broken only at newlines so that all blanks and tabs are
This one is safe, because mbp is based on mbuf, which is on the stack. Index: exec.c =================================================================== RCS file: /cvs/src/bin/csh/exec.c,v retrieving revision 1.19 diff -u -p -r1.19 exec.c --- exec.c 26 Dec 2015 13:48:38 -0000 1.19 +++ exec.c 15 Sep 2018 10:41:24 -0000 @@ -137,7 +137,7 @@ doexec(Char **v, struct command *t) blkfree(pv); pexerr(); } - slash = any(short2str(expath), '/'); + slash = (bool) strchr(short2str(expath), '/'); This one is safe, because expath is set right before via Strsave and 3 lines before it already assumes expath to be a valid pointer. /* * Glob the argument list, if necessary. Otherwise trim off the quote bits. @@ -492,7 +492,7 @@ iscommand(Char *name) Char **pv; Char *sav; struct varent *v; - bool slash = any(short2str(name), '/'); + bool slash = (bool) strchr(short2str(name), '/'); int hashval = 0, hashval1, i; v = adrof(STRpath); @@ -680,7 +680,7 @@ tellmewhat(struct wordent *lexp, Char *s if ((i = iscommand(sp->word)) != 0) { Char **pv; struct varent *v; - bool slash = any(short2str(sp->word), '/'); + bool slash = (bool) strchr(short2str(sp->word), '/'); v = adrof(STRpath); if (v == 0 || v->vec[0] == 0 || slash) The two should be safe, because they're both called with sp->word from tellmewhat(), which is set via globone just prior to iscommand. Since globone is called with G_IGNORE handleone() always returns a valid pointer through Strsave. Other return cases in globone check the validity of the pointer and return it, or create a new string via Strsave(STRNULL). Index: lex.c =================================================================== RCS file: /cvs/src/bin/csh/lex.c,v retrieving revision 1.25 diff -u -p -r1.25 lex.c --- lex.c 30 Aug 2017 07:54:54 -0000 1.25 +++ lex.c 15 Sep 2018 10:41:24 -0000 @@ -957,7 +957,7 @@ domod(Char *cp, int type) case 'h': case 't': - if (!any(short2str(cp), '/')) + if (!strchr(short2str(cp), '/')) return (type == 't' ? Strsave(cp) : 0); wp = Strend(cp); while (*--wp != '/') This function is called from 2 other functions: Through subword twice, which is called twice from dosub: First call to subword is safe, because en->word is checked before calling the function. The second call input is derived from the return of the first call to subword and subword itself makes sure that it does not return a NULL-pointer through Strsave. Through setDolp is safe, because on the first iteration cp is ensured via Strsave. Next increments are assured to have a valid pointer, because of the break on line 754.