> Date: Fri, 5 Feb 2016 13:06:56 -0500
> From: Michael McConville <[email protected]>
>
> Michael McConville wrote:
> > 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?
>
> Is anyone willing to work with me on this? This cleanup should obviously
> be done very carefully, but I think it's worth doing because this code
> is... unfortunate. I'd be happy to submit and review in smaller chunks.
What bug is this fixing? I'm really not interested in diffs that are
basically just churn.
> > > 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:
> > > /*
> > >
> >
>
>
