On Thu, Nov 07, 2013 at 11:32:48AM -0500, Ted Unangst wrote:

> On Thu, Nov 07, 2013 at 17:19, Peter J. Philipp wrote:
> 
> >> +       gid = getgid();
> >> +
> >> +       if (setgroups(1, &gid) == -1)
> >> +               err(1, "setgroups");
> >> +
> >> +       if (setresgid(gid, gid, gid) == -1)
> >> +               err(1, "setresgid");
> >> +
> >>         if (setresuid(uid, uid, uid) == -1)
> >>                 err(1, "setresuid");
> >>
> > 
> > 
> > I thought about it and thought my patch didn't really do anything.  So
> 
> Right. This doesn't do anything. traceroute isn't setgid, it has no
> group privileges to revoke.
> 
> 
> > /* DiffServ Codepoints and other TOS mappings */
> > +       /* KEEP SORTED */
> > const struct toskeywords {
> > const char      *keyword;
> > int              val;
> > @@ -1258,14 +1268,13 @@ map_tos(char *s, int *val)
> > { NULL,                 -1 },
> > };
> > 
> > -       for (t = toskeywords; t->keyword != NULL; t++) {
> > -               if (strcmp(s, t->keyword) == 0) {
> > -                       *val = t->val;
> > -                       return (1);
> > -               }
> > -       }
> > +       t = bsearch(s, toskeywords, nitems(toskeywords), sizeof(struct
> > toskeywords), (int (*)(const void *, const void *))strcmp);
> 
> I don't like the way this is abusing types. In fact, I don't think this
> even works. Did you test it? A pointer to a struct toskeyword will not
> have the same value as the keyword member.

The first field of a struct has the same address as the the struct
itself. Still I consider this bad form and overkill. 

        -Otto

Reply via email to