Re: [libvirt] [PATCH 4/4] storage: fix uid_t|gid_t handling on 32 bit Linux

On 02/22/2013 09:55 AM, Philipp Hahn wrote: I like the standardized name INT_MAX better than the invented name ALLONE. Eww - why do we need to use a double? Just parse to a 32-bit int, then check for truncation after the fact (as I said elsewhere in the series, we already do a compile-time verify that uid_t is not larger than int; so the real problem is if uid_t is smaller than int). Not accurate. mingw64 has s64 pid_t (not u64); and completely lacks uid_t and gid_t natively: $ printf '#include <sys/types.h>\nuid_t\n' | \ x86_64-w64-mingw32-gcc -E -|grep id_t typedef long long _pid_t; typedef _pid_t pid_t; uid_t When using gnulib (as libvirt does), gnulib provides [ug]id_t as 32-bit types even on mingw64. Furthermore, a lot of the problems stem from the fact that s16 or u16 uid_t/gid_t have been used in the past (u16 promotes to s32 without sign extension, so it does not compare equal to (s32)-1). I like the idea of outputting -1 rather than the unsigned all-ones value; which means you need to respin this patch to avoid testsuite failures where we change the test output. Also, anywhere that the parser doesn't accept -1, we need to fix that; accepting -1 as the only valid negative value and requiring all other values to be non-negative makes sense. Looking forward to v2; this patch is appropriate to get into the 1.0.3 release if we get it respun in time. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org