[Toybox] I aten't dead.

[Rich Felker]

On Tue, Feb 24, 2015 at 01:50:03PM -0800, enh wrote:
> On Tue, Feb 24, 2015 at 1:48 PM, Rich Felker <dalias at libc.org> wrote:
> > On Tue, Feb 24, 2015 at 01:30:20PM -0800, enh wrote:
> >> On Tue, Feb 24, 2015 at 11:46 AM, Rob Landley <rob at landley.net> wrote:
> >> > Sorry for the radio silence, $DAYJOB's been eating the majority of my
> >> > programming time and what's left has gone to Aboriginal Linux recently,
> >> > because I made a largeish design change over there (putting the base
> >> > root filesystem in initmpfs and merging the native-compiler at runtime
> >> > instead of compile time), and the aboriginal linux release soft-blocks
> >> > the toybox release because I use "built aboriginal and then built linux
> >> > from scratch under it" as my main toybox regression-smoketest.
> >> >
> >> > Last night's I hit a fun bug in toybox "stat" where it doesn't remotely
> >> > work on arm. (Works fine on x86 host, but 3.18 kernel built against
> >> > uClibc on armv5l: the -f numbers are way off, it thinks the blocksize of
> >> > ext2fs is 32 bytes.)
> >>
> >> seems fine on armv7 3.10 with bionic. oh, specifically the -f output.
> >> yeah, that looks wrong :-)
> >>
> >> a lot of those fields are actually 64-bit, so you need an extra 'l' on LP32:
> >>
> >> diff --git a/toys/other/stat.c b/toys/other/stat.c
> >> index d603316..a96c1de 100644
> >> --- a/toys/other/stat.c
> >> +++ b/toys/other/stat.c
> >> @@ -106,11 +106,11 @@ static void print_stat(char type)
> >>  static void print_statfs(char type) {
> >>    struct statfs *statfs = (struct statfs *)&TT.stat;
> >>
> >> -  if (type == 'a') xprintf("%lu", statfs->f_bavail);
> >> -  else if (type == 'b') xprintf("%lu", statfs->f_blocks);
> >> -  else if (type == 'c') xprintf("%lu", statfs->f_files);
> >> -  else if (type == 'd') xprintf("%lu", statfs->f_ffree);
> >> -  else if (type == 'f') xprintf("%lu", statfs->f_bfree);
> >> +  if (type == 'a') xprintf("%llu", statfs->f_bavail);
> >> +  else if (type == 'b') xprintf("%llu", statfs->f_blocks);
> >> +  else if (type == 'c') xprintf("%llu", statfs->f_files);
> >> +  else if (type == 'd') xprintf("%llu", statfs->f_ffree);
> >> +  else if (type == 'f') xprintf("%llu", statfs->f_bfree);
> >>    else if (type == 'l') xprintf("%ld", statfs->f_namelen);
> >>    else if (type == 't') xprintf("%lx", statfs->f_type);
> >>    else if (type == 'i')
> >
> > Rather than hard-coding an assumption that these have type unsigned
> > long long, the code should be casting them to unsigned long long or
> > uintmax_t and using an appropriate format specifier. I prefer
> > uintmax_t since it's idiomatic and can be used for formatting any
> > unsigned type, but I know Rob can be allergic to these nice semantic
> > types... :-)
> 
> yeah, i almost said the same but because i know that you can't even
> rely on the signedness being the same between different architectures
> i didn't want to get into that argument :-)

I wouldn't worry about the signedness unless negative values are
actually meaningful. Here they're not so an unsigned type is safe; the
full positive range of any type fits in uintmax_t.

Rich

 1424815182.0