[PATCH] fix addgroup command line parsing -- now RFC

[Denis Vlasenko]

On 7/29/07, Tito <farmatito at tiscali.it> wrote:
> On Sunday 29 July 2007 16:27:23 Tito wrote:
> > Hi,
> >
> > addgroup actually does:
> >
> > addgroup   group
> > addgroup   -g num group
> > addgroup   user group
> >
> > so:
> >
> > addgroup -g num user group
> >
> > should not be permitted.
> > This patch fixes the problem for me.
> >
> > diff -uN loginutils/addgroup.c.orig loginutils/addgroup.c
> > --- loginutils/addgroup.c.orig  2007-05-30 14:41:30.000000000 +0200
> > +++ loginutils/addgroup.c       2007-07-29 16:22:50.000000000 +0200
> > @@ -148,6 +148,10 @@
> >         if (argc == 2) {
> >                 struct group *gr;
> >
> > +               /* There was -g on the commandline: error out */
> > +               if (gid)
> > +                       bb_show_usage();
> > +
> >                 /* check if group and user exist */
> >                 xuname2uid(argv[0]); /* unknown user: exit */
> >                 xgroup2gid(argv[1]); /* unknown group: exit */
> >
> >
> > Only little tested.
> > Please apply if you like it.
> >
> > Ciao,
> > Tito
> >
> >
>
> BTW:
>
> as gid_t gid  = 0 has a special meaning as flag in addgroup.c
> (in the xgroup_study function) maybe we should change:
>
>         gid = xatoul_range(group, 0, (gid_t)ULONG_MAX);
> to
>         gid = xatoul_range(group, 1, (gid_t)ULONG_MAX);
>
> so that -g 0 is forbidden.

Which is wrong. gid 0 is technically valid gid.

(gid_t)ULONG_MAX is also wrong, as it results in gid -1.

> OTOH setting the default value gid_t gid = -1
> could also do the trick, but needs some minor
> changes in the code. In this case -g 0 would be
> a legal option. Hints?

Checking for gid being !0 is silly. You can check
whether -g option actually was there, or not.

I committed some fixes to svn, including your fix and
fix which (hopefully) makes -g 0 work correctly.
--
vda