Re: [PATCH] corrected setenv|putenv rc checking (bin)

["Simon 'corecode' Schubert" <corecode@xxxxxxxxxxxx>]

Alexey Slynko wrote:
> > > Index: ls/ls.c
> > > ===================================================================
> > > RCS file: /mnt/share/dragonfly-cvs/src/bin/ls/ls.c,v
> > > retrieving revision 1.14
> > > diff -u -r1.14 ls.c
> > > --- ls/ls.c    18 Sep 2005 18:01:49 -0000    1.14
> > > +++ ls/ls.c    29 Sep 2005 00:48:06 -0000
> > > @@ -221,7 +221,8 @@
> > >              fts_options |= FTS_COMFOLLOW;
> > >              break;
> > >          case 'G':
> > > -            setenv("CLICOLOR", "", 1);
> > > +            if (setenv("CLICOLOR", "", 1) != 0)
> > > +                err(1, "setenv: cannot set CLICOLOR");
> > >              break;
> > >          case 'L':
> > >              fts_options &= ~FTS_PHYSICAL;
> >
> >
> > I still think this shouldn't abort.
>
> I still think that ls is not an exception of rule

Abort ls because it can't color the output?  This is absolutely 
ridiculous.  Maybe invent a global int clicolor?

> > > Index: sh/var.c
> > > ===================================================================
> > > RCS file: /mnt/share/dragonfly-cvs/src/bin/sh/var.c,v
> > > retrieving revision 1.8
> > > diff -u -r1.8 var.c
> > > --- sh/var.c    19 Apr 2005 05:18:19 -0000    1.8
> > > +++ sh/var.c    29 Sep 2005 01:02:35 -0000
> > > @@ -316,7 +317,8 @@
> > >              if (vp == &vmpath || (vp == &vmail && ! mpathset()))
> > >                  chkmail(1);
> > >              if ((vp->flags & VEXPORT) && localevar(s)) {
> > > -                putenv(s);
> > > +                if (putenv(s) != 0)
> > > +                    err(1, "putenv: cannot set %s", s);
> > >                  setlocale(LC_ALL, "");
> > >              }
> > >              INTON;
> > > @@ -332,7 +334,8 @@
> > >      INTOFF;
> > >      *vpp = vp;
> > >      if ((vp->flags & VEXPORT) && localevar(s)) {
> > > -        putenv(s);
> > > +        if (putenv(s) != 0)
> > > +            err(1, "putenv: cannot set %s", s);
> > >          setlocale(LC_ALL, "");
> > >      }
> > >      INTON;
> > > @@ -553,7 +556,8 @@
> > >                           vp->flags |= flag;
> > >                          if ((vp->flags & VEXPORT) && 
> > > localevar(vp->text)) {
> > > -                            putenv(vp->text);
> > > +                            if (putenv(vp->text) != 0)
> > > +                                err(1, "putenv: cannot set %s", 
> > > vp->text);
> > >                              setlocale(LC_ALL, "");
> > >                          }
> > >                          goto found;
> >
> >
> > no!  don't collapse my shell if it can't allocate an env var!  set $? 
> > and output a warning.  You don't expect the shell to collapse if it 
> > can't exec a binary, right?
> >
> > $ /bin/ehco foo
> > /bin/ehco: Command not found.
> >
> > Login: _
> >
> > WTF?
>
> Can't reproduce it. Can you explain, how it can be reproduced?

This is an example.  You wouldn't want sh to close itself if it can't 
run the binary you are asking it to run.  Following this reasoning you'd 
not want the shell to close itself if it can't set the env var you are 
asking it to set.

--
Serve - BSD     +++  RENT this banner advert  +++    ASCII Ribbon   /"\
Work - Mac      +++  space for low $$$ NOW!1  +++      Campaign     \ /
Party Enjoy Relax   |   http://dragonflybsd.org      Against  HTML   \
Dude 2c 2 the max   !   http://golden-apple.biz       Mail + News   / \