[Toybox] [PATCH] chown/chgrp with unknown numeric ids

Rob Landley rob at landley.net
Sat Aug 20 13:46:30 PDT 2016


On 08/20/2016 06:01 AM, darken wrote:
>     I was grinding through my todo list last night and fixed this, and only
>     just noticed replying to your message that you'd attached a patch.
>     (Oops. Sorry.)
> 
> 
> :-|, well at least it's fixed.
> 
>     The fixes look fairly similar. I need to update tests/chown.test and
> 
>     tests/chgrp.test to actually check for this usefully. (Poking at
>     that now.)
> 
> 
> Why return `unsigned` instead of `uid_t`/`gid_t` from the get uid/gid
> methods?

Because it's hard to range check otherwise.

Unsigned int is the underlying type in uid_t these days (there's a
getuid32 and everything), and if somebody tries to select a numeric uid
out of range I want to catch that rather than wrapping it. I dunno off
the top of my head what the UID_T_MAX macros would be (didn't find one
in
http://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/stat.h.html
and similar), and 1LL<<8*sizeof(x) is just awkward (and assumes it's
unsigned, which it _is_ but I'm not sure it's _required_ to be?)

I have a tendency to squash things to known types and operate on them,
producing behavior that's at least consistent. Possibly inappropriate in
this case...

On old Linux systems you had 16 bit uid ranges, but that went away
before the posix 2008 (openat) stuff went in, so I think we already
don't support such systems. Ok, it's still a build config option in the
kernel, CONFIG_UID16, but so is CONFIG_BINFMT_AOUT a.out binary support.

And then there's the discussion of 65534 being the _other_ magic UID
number in the kernel. uid is 0, nobody is 65534, and the kernel knows
both: https://lwn.net/Articles/695478/

Legacy from the 16 bit UID days.

If it really bothers you, I can change it back. It just makes the range
test... sort of inconclusive.

> By the way, I have another patch for the test applet:
> https://github.com/landley/toybox/pull/47
> <https://github.com/landley/toybox/pull/47>

Test is in pending for a reason, but I'll take a look at this after I
finish adding tunctl and promoting getfattr/setfattr. (Much reading on
how xattrs work. Whoever designed these APIs should be harmed.)

Thanks,

Rob



More information about the Toybox mailing list