3:43 a.m.
*** BLURB HERE ***
Michal Privoznik (2):
utils: Return proper value for virGet{User,Group}ID
utiltest: Don't assume 'char' is always signed
src/util/virutil.c | 4 ++--
tests/utiltest.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
--
2.3.6
3:43 a.m.
New subject: [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 <mprivozn(a)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
3:47 a.m.
New subject: [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 <mprivozn(a)redhat.com>
---
src/util/virutil.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
ACK
3:57 a.m.
New subject: [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 <mprivozn(a)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
3:43 a.m.
New subject: [libvirt] [PATCH 2/2] utiltest: Don't assume 'char' is always signed
Not every architecture out there has 'char' signed by default.
For instance, my arm box has it unsigned by default:
$ gcc -dM -E - < /dev/null | grep __CHAR_UNSIGNED__
#define __CHAR_UNSIGNED__ 1
Therefore, after 65c61e50 the test if failing for me. Problem is,
we are trying to assign couple of negative values into char
assuming some will overflow and some don't. That can't be the
case if 'char' is unsigned by default.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/utiltest.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/utiltest.c b/tests/utiltest.c
index 3a1f8eb..9eb7fb6 100644
--- a/tests/utiltest.c
+++ b/tests/utiltest.c
@@ -185,7 +185,7 @@ testOverflowCheckMacro(const void *data ATTRIBUTE_UNUSED)
{
long long tmp;
unsigned char luchar;
- char lchar;
+ signed char lchar;
TEST_OVERFLOW(luchar, 254, false);
TEST_OVERFLOW(luchar, 255, false);
--
2.3.6
4:02 a.m.
New subject: [libvirt] [PATCH 2/2] utiltest: Don't assume 'char' is always signed
On Mon, Jun 08, 2015 at 10:43:39 +0200, Michal Privoznik wrote:
Not every architecture out there has 'char' signed by
default.
For instance, my arm box has it unsigned by default:
$ gcc -dM -E - < /dev/null | grep __CHAR_UNSIGNED__
#define __CHAR_UNSIGNED__ 1
Therefore, after 65c61e50 the test if failing for me. Problem is,
we are trying to assign couple of negative values into char
assuming some will overflow and some don't. That can't be the
case if 'char' is unsigned by default.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/utiltest.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/utiltest.c b/tests/utiltest.c
index 3a1f8eb..9eb7fb6 100644
--- a/tests/utiltest.c
+++ b/tests/utiltest.c
@@ -185,7 +185,7 @@ testOverflowCheckMacro(const void *data ATTRIBUTE_UNUSED)
{
long long tmp;
unsigned char luchar;
- char lchar;
+ signed char lchar;
I actually did not know that without explicit specification char may be
signed or unsigned. Anyways, since char is not defined I'd rather change
the type to uint8_t and int8_t.
Peter
3447
days inactive
3447
days old
5 comments
3 participants
participants (3)
-
Martin Kletzander -
Michal Privoznik -
Peter Krempa