Re: [libvirt] [PATCH 1/2] utils: Return proper value for virGet{User, Group}ID
On Mon, Jun 08, 2015 at 10:43:38AM +0200, Michal Privoznik wrote: These two functions are used to translate user or group name into a numerical ID. Depending on platform we are building for, we have an implementation for UNIX-like systems, and a stub implementation for Windows. While the former returns a negative value on error, the latter simply reports an error (saying something about missing implementation) and returns the value of zero. This makes the caller think function did succeed and passed variable had been set to the correct value. Well, it was not. Even compiler spots this when compiling for win32: CC util/libvirt_util_la-virutil.lo ../../src/util/virutil.c: In function 'virParseOwnershipIds': ../../src/util/virutil.c:2410:17: error: 'theuid' may be used uninitialized in this function [-Werror=maybe-uninitialized] *uidPtr = theuid; ^ ../../src/util/virutil.c:2380:9: note: 'theuid' was declared here uid_t theuid; ^ ../../src/util/virutil.c:2412:17: error: 'thegid' may be used uninitialized in this function [-Werror=maybe-uninitialized] *gidPtr = thegid; ^ ../../src/util/virutil.c:2381:9: note: 'thegid' was declared here gid_t thegid; ^ cc1: all warnings being treated as errors Makefile:9167: recipe for target 'util/libvirt_util_la-virutil.lo' failed Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/util/virutil.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) ACK signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] utils: Return proper value for virGet{User, Group}ID
These two functions are used to translate user or group name into a numerical ID. Depending on platform we are building for, we have an implementation for UNIX-like systems, and a stub implementation for Windows. While the former returns a negative value on error, the latter simply reports an error (saying something about missing implementation) and returns the value of zero. This makes the caller think function did succeed and passed variable had been set to the correct value. Well, it was not. Even compiler spots this when compiling for win32: CC util/libvirt_util_la-virutil.lo ../../src/util/virutil.c: In function 'virParseOwnershipIds': ../../src/util/virutil.c:2410:17: error: 'theuid' may be used uninitialized in this function [-Werror=maybe-uninitialized] *uidPtr = theuid; ^ ../../src/util/virutil.c:2380:9: note: 'theuid' was declared here uid_t theuid; ^ ../../src/util/virutil.c:2412:17: error: 'thegid' may be used uninitialized in this function [-Werror=maybe-uninitialized] *gidPtr = thegid; ^ ../../src/util/virutil.c:2381:9: note: 'thegid' was declared here gid_t thegid; ^ cc1: all warnings being treated as errors Makefile:9167: recipe for target 'util/libvirt_util_la-virutil.lo' failed Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/util/virutil.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index e479cce..cddc78a 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1316,7 +1316,7 @@ int virGetUserID(const char *name ATTRIBUTE_UNUSED, virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(virGetUserID is not available)); -return 0; +return -1; } @@ -1326,7 +1326,7 @@ int virGetGroupID(const char *name ATTRIBUTE_UNUSED, virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(virGetGroupID is not available)); -return 0; +return -1; } int -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] utils: Return proper value for virGet{User, Group}ID
On Mon, Jun 08, 2015 at 10:47:05 +0200, Martin Kletzander wrote: On Mon, Jun 08, 2015 at 10:43:38AM +0200, Michal Privoznik wrote: These two functions are used to translate user or group name into a numerical ID. Depending on platform we are building for, we have an implementation for UNIX-like systems, and a stub implementation for Windows. While the former returns a negative value on error, the latter simply reports an error (saying something about missing implementation) and returns the value of zero. This makes the caller think function did succeed and passed variable had been set to the correct value. Well, it was not. Even compiler spots this when compiling for win32: CC util/libvirt_util_la-virutil.lo ../../src/util/virutil.c: In function 'virParseOwnershipIds': ../../src/util/virutil.c:2410:17: error: 'theuid' may be used uninitialized in this function [-Werror=maybe-uninitialized] *uidPtr = theuid; ^ ../../src/util/virutil.c:2380:9: note: 'theuid' was declared here uid_t theuid; ^ ../../src/util/virutil.c:2412:17: error: 'thegid' may be used uninitialized in this function [-Werror=maybe-uninitialized] *gidPtr = thegid; ^ ../../src/util/virutil.c:2381:9: note: 'thegid' was declared here gid_t thegid; ^ cc1: all warnings being treated as errors Makefile:9167: recipe for target 'util/libvirt_util_la-virutil.lo' failed Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/util/virutil.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) ACK I actually pushed the same patch a while ago. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
