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.

Reply via email to