On (23/11/15 15:29), Lukas Slebodnik wrote:
On (20/11/15 10:17), Lukas Slebodnik wrote:
>ehlo,
>
>I found few warnings on rawhide when I build sssd-1.13.2
>https://kojipkgs.fedoraproject.org//work/tasks/8618/11918618/build.log
>
>LS
I missed one warning in pysss_nss_idmap.cwhich was not visible
on 64 bit architecture.
@see 32 bit build log
https://kojipkgs.fedoraproject.org//packages/sssd/1.13.2/1.fc24/data/logs...
BTW warning are caused by newly added warnings in python3.5 cflags
PYTHON3_CFLAGS='-I/usr/include/python3.5m -I/usr/include/python3.5m
-Wno-unused-result -Wsign-compare -Wunreachable-code -O2 -g -pipe -Wall
-Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches
-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m32 -march=i686 -mtune=atom
-fasynchronous-unwind-tables -D_GNU_SOURCE -fPIC -fwrapv
-DDYNAMIC_ANNOTATIONS_ENABLED=1 -DNDEBUG -O2 -g -pipe -Wall
-Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches
-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m32 -march=i686 -mtune=atom
-fasynchronous-unwind-tables -D_GNU_SOURCE -fPIC -fwrapv'
python3.5 is only on rawhide.
Updated patches are attached.
LS
From c3e22efaacac525476ade16fd7d81bdfe7115e5c Mon Sep 17 00:00:00
2001
From: Lukas Slebodnik <lslebodn(a)redhat.com>
Date: Thu, 19 Nov 2015 13:07:10 +0000
Subject: [PATCH 1/3] TOOLS: Fix warning Wsign-compare
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
src/tools/tools_util.c: In function ‘parse_groups’:
src/tools/tools_util.c:116:19: error: comparison between
signed and unsigned integer expressions [-Werror=sign-compare]
for (i = 0; i < tokens; i++) {
^
---
src/tools/tools_util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tools/tools_util.c b/src/tools/tools_util.c
index f9dca728751f0429bb2f1c3ef46ffc269ecfba40..f475a51ef380f5d0397deb22d161fd2c8d2a81b8
100644
--- a/src/tools/tools_util.c
+++ b/src/tools/tools_util.c
@@ -94,7 +94,7 @@ int parse_groups(TALLOC_CTX *mem_ctx, const char *optstr, char ***_out)
char *orig, *n, *o;
char delim = ',';
unsigned int tokens = 1;
- int i;
+ unsigned int i;
orig = talloc_strdup(mem_ctx, optstr);
if (!orig) return ENOMEM;
--
2.5.0
From b79f1fe6f6ffec7b9a7c9f045c12bc6c894faa78 Mon Sep 17 00:00:00
2001
From: Lukas Slebodnik <lslebodn(a)redhat.com>
Date: Thu, 19 Nov 2015 13:07:52 +0000
Subject: [PATCH 2/3] pysss_murmur: Fix warning Wsign-compare
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
src/python/pysss_murmur.c: In function ‘py_murmurhash3’:
src/python/pysss_murmur.c:47:17: error: comparison between
signed and unsigned integer expressions [-Werror=sign-compare]
key_len > strlen(key)) {
^
uint32_t murmurhash3(const char *key, int len, uint32_t seed)
The second argument of the function murmurhash3 has type int.
But the code expects to be unsigned integer.
There is code in python wrapper py_murmurhash3
which check boundaries of that argument.
It should be an unsigned "key_len > INT_MAX || key_len < 0".
An exception should be thrown for negative number.
Moreover, the length should be shorter then a length of input string.
The strlen returns size_t which is unsigned and key_len is signed long.
We already checked that value is unsigned so
we can safely cast key_len to size_t
---
src/python/pysss_murmur.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/python/pysss_murmur.c b/src/python/pysss_murmur.c
index 97d752b2a7734a332a5d5da07d75b594638015c8..b14a672025c218ae3ab314c3ad2cf2c5ced40870
100644
--- a/src/python/pysss_murmur.c
+++ b/src/python/pysss_murmur.c
@@ -44,7 +44,7 @@ static PyObject * py_murmurhash3(PyObject *module, PyObject *args)
}
if (seed > UINT32_MAX || key_len > INT_MAX || key_len < 0 ||
- key_len > strlen(key)) {
+ (size_t)key_len > strlen(key)) {
PyErr_Format(PyExc_ValueError, "Invalid value\n");
return NULL;
}
--
2.5.0
From 07d5ea89b83e0176f7e51bd8b0ae9881aadff3bf Mon Sep 17 00:00:00
2001
From: Lukas Slebodnik <lslebodn(a)redhat.com>
Date: Thu, 19 Nov 2015 14:17:36 +0000
Subject: [PATCH 3/3] pyhbac: Fix warning Wsign-compare
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
src/python/pyhbac.c: In function ‘HbacRuleElement_repr’:
src/python/pyhbac.c:506:59: error: comparison between
signed and unsigned integer expressions [-Werror=sign-compare]
if (strnames == NULL || strgroups == NULL || category == -1) {
^
src/python/pyhbac.c: In function ‘HbacRuleElement_to_native’:
src/python/pyhbac.c:614:51: error: comparison between
signed and unsigned integer expressions [-Werror=sign-compare]
if (!el->names || !el->groups || el->category == -1) {
^
The static function native_category had type of terurn value uint32_t
But it also could return -1 which indicated an error.
It's better to don't mix return code with returned value.
---
src/python/pyhbac.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/src/python/pyhbac.c b/src/python/pyhbac.c
index fca476c56ba697693c793ecad7945ae7edf3dffd..820ef11b57f1226725fd7acf97598a42e6bf0bc0
100644
--- a/src/python/pyhbac.c
+++ b/src/python/pyhbac.c
@@ -191,8 +191,8 @@ pyobject_to_category(PyObject *o)
return -1;
}
-static uint32_t
-native_category(PyObject *pycat)
+static int
+native_category(PyObject *pycat, uint32_t *_category)
{
PyObject *iterator;
PyObject *item;
@@ -218,7 +218,9 @@ native_category(PyObject *pycat)
}
Py_DECREF(iterator);
- return cat;
+
+ *_category = cat;
+ return 0;
}
static char *
@@ -491,6 +493,7 @@ HbacRuleElement_repr(HbacRuleElement *self)
char *strnames = NULL;
char *strgroups = NULL;
uint32_t category;
+ int ret;
PyObject *o, *format, *args;
format = PyUnicode_FromString("<category %lu names [%s] groups
[%s]>");
@@ -502,8 +505,8 @@ HbacRuleElement_repr(HbacRuleElement *self)
discard_const_p(char, ","));
strgroups = str_concat_sequence(self->groups,
discard_const_p(char, ","));
- category = native_category(self->category);
- if (strnames == NULL || strgroups == NULL || category == -1) {
+ ret = native_category(self->category, &category);
+ if (strnames == NULL || strgroups == NULL || ret == -1) {
PyMem_Free(strnames);
PyMem_Free(strgroups);
Py_DECREF(format);
@@ -592,6 +595,7 @@ struct hbac_rule_element *
HbacRuleElement_to_native(HbacRuleElement *pyel)
{
struct hbac_rule_element *el = NULL;
+ int ret;
/* check the type, None would wreak havoc here because for some reason
* it would pass the sequence check */
@@ -608,10 +612,10 @@ HbacRuleElement_to_native(HbacRuleElement *pyel)
goto fail;
}
- el->category = native_category(pyel->category);
+ ret = native_category(pyel->category, &el->category);
el->names = sequence_as_string_list(pyel->names, "names");
el->groups = sequence_as_string_list(pyel->groups, "groups");
- if (!el->names || !el->groups || el->category == -1) {
+ if (!el->names || !el->groups || ret == -1) {
goto fail;
}
--
2.5.0
From 8612123c0c57db3dcb4e7138e299ab0f8f5e4ebb Mon Sep 17 00:00:00
2001
From: Lukas Slebodnik <lslebodn(a)redhat.com>
Date: Mon, 23 Nov 2015 14:17:05 +0000
Subject: [PATCH 4/4] pysss_nss_idmap: Fix warning Wsign-compare
We get id from python with type long.
We need to checked if we can safely cast to unsigned 32 bit integer.
src/python/pysss_nss_idmap.c: In function 'do_getsidbyid':
src/python/pysss_nss_idmap.c:155:22: warning:
comparison between signed and unsigned integer expressions [-Wsign-compare]
if (id < 0 || id > UINT32_MAX) {
^
NACK to 4th patch.
I would say it's but in do_getsidbyid.
The size of "long" is 4 bytes on i686.
It's the same size as size of "int"
So we filter out ID higher than 1073741824 on i686.
LS