On 11/07/13 17:48, Otto Moerbeek wrote:
> 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


Hi,

while I don't want to persue this patch further, I'd like to say that I
finished it on my own, thanks to your input I understand what base in
bsearch() is supposed to be now.  I had something in mind from qsort()
which also has a variable called base in the manpages and that had
confused me.  I have taken a look how bsearch() in other programs and
I have noticed that some are doing it like me but wrap strcmp inside
another *cmp where there is a bit of casting being done.  I'm wondering
if that is the right way?  Or if it can be cleaned up?

Thanks!

-peter

Reply via email to