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/i686/build.log > >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 <[email protected]> >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 <[email protected]> >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 <[email protected]> >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 <[email protected]> >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 _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
