Michael McConville wrote: > Maxim Pugachev wrote: > > In a case when the shell name is not specified (i.e. just "#!" without > > a path), don't run the heavy logic that checks shell, simply return > > ENOENT. > > I'm not sure whether this is a reasonable thing to do. Someone with more > kernel API experience will have to comment. > > > Also, as a tiny improvement: avoid a loop when calculating shell's > > args length. > > It seems like there's a simpler solution to this: strlen. Also, the > assignment that follows the for loop seems dead, as we know *cp will be > NUL. > > The below diff makes that change, removes the useless if condition you > noticed, and bumps a few variable initializations into the declaration > block.
I just remembered that this was probably a perfomance concern as well. In that case, we could assign shellarg and shellarglen earlier in the function, right? > Index: sys/kern/exec_script.c > =================================================================== > RCS file: /cvs/src/sys/kern/exec_script.c,v > retrieving revision 1.37 > diff -u -p -r1.37 exec_script.c > --- sys/kern/exec_script.c 31 Dec 2015 18:55:33 -0000 1.37 > +++ sys/kern/exec_script.c 10 Jan 2016 01:41:55 -0000 > @@ -67,9 +67,9 @@ > int > exec_script_makecmds(struct proc *p, struct exec_package *epp) > { > - int error, hdrlinelen, shellnamelen, shellarglen; > + int error, hdrlinelen, shellnamelen, shellarglen = 0; > char *hdrstr = epp->ep_hdr; > - char *cp, *shellname, *shellarg, *oldpnbuf; > + char *cp, *shellname = NULL, *shellarg = NULL, *oldpnbuf; > char **shellargp = NULL, **tmpsap; > struct vnode *scriptvp; > uid_t script_uid = -1; > @@ -110,10 +110,6 @@ exec_script_makecmds(struct proc *p, str > if (cp >= hdrstr + hdrlinelen) > return ENOEXEC; > > - shellname = NULL; > - shellarg = NULL; > - shellarglen = 0; > - > /* strip spaces before the shell name */ > for (cp = hdrstr + EXEC_SCRIPT_MAGICLEN; *cp == ' ' || *cp == '\t'; > cp++) > @@ -122,8 +118,6 @@ exec_script_makecmds(struct proc *p, str > /* collect the shell name; remember its length for later */ > shellname = cp; > shellnamelen = 0; > - if (*cp == '\0') > - goto check_shell; > for ( /* cp = cp */ ; *cp != '\0' && *cp != ' ' && *cp != '\t'; cp++) > shellnamelen++; > if (*cp == '\0') > @@ -142,9 +136,8 @@ exec_script_makecmds(struct proc *p, str > * behaviour. > */ > shellarg = cp; > - for ( /* cp = cp */ ; *cp != '\0'; cp++) > - shellarglen++; > - *cp++ = '\0'; > + shellarglen = strlen(shellarg); > + cp += shellarglen + 1; > > check_shell: > /* >