On Tue, Feb 24, 2015 at 01:50:03PM -0800, enh wrote: > On Tue, Feb 24, 2015 at 1:48 PM, Rich Felker <[email protected]> wrote: > > On Tue, Feb 24, 2015 at 01:30:20PM -0800, enh wrote: > >> On Tue, Feb 24, 2015 at 11:46 AM, Rob Landley <[email protected]> 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 _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
