[Toybox] Patches for toybox-0.4.8

[Rob Landley]

On 06/24/14 16:20, Isaac Dunham wrote:
> On Tue, Jun 24, 2014 at 04:54:57PM +0200, luckboy at vp.pl wrote:
>> W dniu 23.06.2014 23:45, Isaac Dunham pisze:
>>> On Mon, Jun 23, 2014 at 05:35:55PM +0200, luckboy at vp.pl wrote:
>>>> I created own build system for small linux distributions. I had to
>>>> write two patches for your
>>>> toybox-0.4.8. First patch fixes display of the id toy. Second patch
>>>> fixes sigsegv of the which
>>>> toy for the unset PATH variable of environment. These patches:
>>>>
>>>> https://github.com/luckboy/toyroot/blob/master/patch/toybox-0.4.8-fixes.patch
>>> This is not correct; a system where ngroups !> 0 is broken, and id *should*
>>> perror_exit().
>>> Make sure you have something in /etc/group and /etc/passwd.
>>
>> But if (0 >= ngroups) also isn't correct because getgrouplist
>> returns -1 for error but doesn't return 0. There should  be if (0 >
>> ngroups) instead of if (0 >= ngroups).
>> Maybe I could changed it at my patch but I wasn't sure reaction of
>> the id toy.
> 
> On any sane *nix system, every user will be a member of at least one group.

Actually I'm starting to lean towards luckboy's point of view on this
one. If we have user information but not group information, we can show
what we've got.

That said, it would be really nice of
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/id.html was of
ANY USE in this situation. Sigh... Ok, in the stdout section it says:

"uid=%u(%s) gid=%u(%s)\n", <real user ID>, <user-name>,
    <real group ID>, <group-name>

I.E. at least one gid is not optional, according to posix? That said,
it's not guaranteed to be _there_. /etc/passwd has a gid field but
nobody said /etc/groups has that entry.

Sigh. My first quick stab at it looks like:

--- a/toys/posix/id.c	Tue Jun 24 21:47:07 2014 -0500
+++ b/toys/posix/id.c	Tue Jun 24 23:37:42 2014 -0500
@@ -113,14 +113,13 @@
   i = sizeof(toybuf)/sizeof(gid_t);
   ngroups = username ? getgrouplist(username, gid, groups, &i)
     : getgroups(i, groups);
-  if (0 >= ngroups) perror_exit(0);
+  if (ngroups<0) perror_exit(0);

-  for (i = 0;;) {
+  for (i = 0; i<ngroups; i++) {
+    if (i) xputc(' ');
     if (!(grp = getgrgid(groups[i]))) perror_msg(0);
     else if (flags & FLAG_G) s_or_u(grp->gr_name, grp->gr_gid, 0);
     else if (grp->gr_gid != egid) showid("", grp->gr_gid, grp->gr_name);
-    if (++i >= ngroups) break;
-    xputc(' ');
   }
   xputc('\n');
 }


The space at the beginning is wrong because if you don't feed in flag -G
it prints the initial group= before the loop, except that called
xgetgrid() which will error_exit() if it can't return one, and if it
_did_ return a null pointer the later users would segfault...

Sigh. Waaay past my bedtime...

Rob

 1403671518.0