[Toybox] [CLEANUP] More thoughts on stat.c cleanup.

[Felix Janda]

On 2013-05-30 at 10:22, Rob Landley wrote:
> Sigh, my idea from the other night didn't pan out. I was hoping that I  
> could always pass a long to xprintf(), and then just store the xprintf  
> format string and a pointer to the value, dereference it as a long *,  
> and let xprintf figure out how much of the stack to consume. (The  
> endianness should work out either way, and padding isn't an issue when  
> you only have one value.)
> 
> The problem is alignment: arm64 can't read a long from an int-aligned  
> address. 8 byte types need 8 byte alignment, not 4. Blah.
> 
> What I really want is some variant of vprintf() that can start reading  
> values from a pointer location instead of a va_list. Unfortunately,  
> libc hasn't got one. Nor has it got "I passed you a pointer,  
> dereference it" notation from the actual printf logic that knows the  
> size of the data type...

Hmm, I should have mentioned that I also had an idea along that line and
would also be happy if there was such a variant of vprintf(). On some
systems va_list should actually be implemented in that way.

> Ok, I need to collate the "long" and "long long" types into two printf  
> groups. And isolate the stuff with extra processing....
> 
> Hang on, this is wrong:
> 
>      case 'X':
>        xprintf("%llu", stat->st_atime);
> 
> That's just "long", not "long long". (In the kernel source, see  
> arch/include/uapi/asm/stat.h, definition of struct stat _and_ struct  
> stat64, it's unsigned long in both. I have no idea why st_mtime_nsec is  
> "int", but int and long are equivalent on 32 bit x86 so it's just weird  
> and not actually incorrect.)

I didn't really check the format strings for correctness. It seemed like
it would get a headache to get them right on all architectures.

> And that's the problem: on a 32 bit system, long is the same size as  
> int, not as uint64_t. How on earth does that work? (Is gcc promoting  
> the type...?)
> 
>    $ CFLAGS="--static -Wall"  
> CROSS_COMPILE=~/cross-compiler-i686/bin/i686- make
>    $ stat -c %Y /
>    1368946682
>    $ ./toybox stat -c %Y /
>    104448161786
> 
> So that's a no, then. It's just buggy.
> 
> Ok, how portable is that type?
> 
>    $ grep "st_mtime;"  arch/*/include/uapi/asm/stat.h \
>      | sed 's/^.*://;s/[ \t][ \t]*/ /g' | sort -u
>    int st_mtime; /* Time of last modification. */
>    signed int st_mtime;
>    time_t st_mtime;
>    unsigned int st_mtime;
>    unsigned long st_mtime;
>    unsigned long st_mtime; /* Time of last modification. */
> 
> Apparently not. What size is time_t anyway? It's __kernel_time_t. Which  
> is...
> 
>    $ find $(find . -name uapi) -name "*.h" | xargs grep __kernel_time_t  
> | grep typedef
>    ./include/uapi/asm-generic/posix_types.h:typedef __kernel_long_t	 
> __kernel_time_t;
>    $ find $(find . -name uapi) -name "*.h" | xargs grep __kernel_long_t  
> | grep typedef
>    ./arch/x86/include/uapi/asm/posix_types_x32.h:typedef long long  
> __kernel_long_t;
>    ./include/uapi/asm-generic/posix_types.h:typedef long		 
> __kernel_long_t;
> 
> They special cased 32 bit x86 and nobody else did. Bravo. So 32 bit arm  
> still has a 2038 problem? (What?)
> 
> Right. Looks like I have to hit it with a typecast, _and_ I might have  
> to check the other types to see if they match up with what printf  
> expects...
> 
> Lemme get back to this one. :)

It sounds like great fun.

Felix