[PATCH v3 3/4] nsenter: new applet

[Mike Frysinger]

On 14 Mar 2016 14:27, Mike Frysinger wrote:
> On 14 Mar 2016 18:11, Bartosz Gołaszewski wrote:
> > 2016-03-14 15:27 GMT+01:00 Mike Frysinger <vapier at gentoo.org>:
> > > On 14 Mar 2016 11:07, Bartosz Golaszewski wrote:
> > >> +#ifndef __BB_NAMESPACE_H
> > >> +#define __BB_NAMESPACE_H
> > >
> > > use a naming style like other busybox headers
> > >
> > >> +/*
> > >> + * Longest possible path to a procfs file used in namespace utils. Must be
> > >> + * able to contain the '/proc/' string, the '/ns/user' string which is the
> > >> + * longest namespace name and a 32-bit integer representing the process ID.
> > >> + */
> > >> +#define NS_PROC_PATH_MAX (sizeof("/proc//ns/user") + sizeof(pid_t) * 3)
> > >
> > > using the sizeof pid_t as a proxy for how many chars it'd take to render
> > > a decimal number in ASCII is wonky.  just hardcode it as "10" since that
> > > is the largest unsigned 32bit number ("4294967296").
> > 
> > Can you elaborate on how it's wonky? While your solution is perfectly
> > fine I think that there's nothing wrong with the way I've done it
> > neither.
> 
> the code seems to assume that the byte size scales into the number of
> chars required to represent the number in base 10 when it's really a
> log scale.  here's a table to show it's wonky:
> 
> pid_t |sizeof |*3 val|actual|
> size  |(bytes)|(char)|      |
> (bits)|       |      |      |
> ------|-------|------|------|
>   8   |  1    |  3   |  4   |
>  16   |  2    |  6   |  6   |
>  32   |  4    | 12   | 11   |
>  64   |  8    | 24   | 20   |
> 128   | 16    | 48   | 40   |
> 
> the "actual" value is one higher than you might expect because pid_t
> is a signed quantity.  for 8bit, "-128" is 0x80 and takes 4 bytes.
> for 32bit ("int"), "-2147483648" is 0x80000000 and takes 11 bytes.
> 
> so my initial suggestion of "10" should actually be "11".

just to be extra clear: i'm not saying your current code produces wrong
behavior using current code/systems -- when pid_t is a 32-bit signed int,
then the padding of 12 bytes is sufficient.  i'm saying the code as is is
written now implies a relationship that does not actually exist.

all that said, i'm sure there is code in busybox already that writes it
this way and would blow up horribly if pid_t was not a 32-bit signed int.
perhaps adding some proc helpers wouldn't be a terrible idea ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20160314/fc516bb7/attachment.asc>