On 02/22/2013 09:55 AM, Philipp Hahn wrote:
uid_t and gid_t are opaque types, ranging from s32 to u32 to u64.
Unfortunately libvirt uses the value -1 to mean "current process", which on Linux32 gets converted to ALLONE := +(2^32-1) = 4294967295.
I like the standardized name INT_MAX better than the invented name ALLONE.
Different libvirt versions used different formatting in the past, which break one or the other parsing function: virXPathLong(): (signed long)-1 != (double)ALLONE virXPathULong(): (unsigned long)-1 != (double)-1
Roll our own version of virXPathULong(), which also accepts -1, which is silently converted to ALLONE.
For output: do the reverse and print -1 instead of ALLONE.
Signed-off-by: Philipp Hahn <hahn@univention.de> --- src/conf/storage_conf.c | 74 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 60 insertions(+), 14 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 9134a22..b267f00 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -665,7 +665,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, const char *permxpath, int defaultmode) { char *mode; - long v; + double d;
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).
int ret = -1; xmlNodePtr relnode; xmlNodePtr node; @@ -699,26 +699,57 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, VIR_FREE(mode); }
+ /* uid_t and gid_t are *opaque* types: on Linux they're u32, on Windows64 + * they're u64, on Solaris they were s32 in the past. There may be others.
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