xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [Xen-devel] [PATCH 1/2] python: fix -Wsign-compare warnings
@ 2019-08-09  2:01 Marek Marczykowski-Górecki
  2019-08-09  2:01 ` [Xen-devel] [PATCH 2/2] tools/tests/cpu-policy: disable -Wformat-overflow Marek Marczykowski-Górecki
  2019-08-09 10:42 ` [Xen-devel] [PATCH 1/2] python: fix -Wsign-compare warnings Ian Jackson
  0 siblings, 2 replies; 10+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-08-09  2:01 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Marek Marczykowski-Górecki, Wei Liu

Specifically:
xen/lowlevel/xc/xc.c: In function ‘pyxc_domain_create’:
xen/lowlevel/xc/xc.c:147:24: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
  147 |         for ( i = 0; i < sizeof(xen_domain_handle_t); i++ )
      |                        ^
xen/lowlevel/xc/xc.c: In function ‘pyxc_domain_sethandle’:
xen/lowlevel/xc/xc.c:312:20: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
  312 |     for ( i = 0; i < sizeof(xen_domain_handle_t); i++ )
      |                    ^
xen/lowlevel/xc/xc.c: In function ‘pyxc_domain_getinfo’:
xen/lowlevel/xc/xc.c:391:24: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
  391 |         for ( j = 0; j < sizeof(xen_domain_handle_t); j++ )
      |                        ^
xen/lowlevel/xc/xc.c: In function ‘pyxc_get_device_group’:
xen/lowlevel/xc/xc.c:677:20: error: comparison of integer expressions of different signedness: ‘int’ and ‘uint32_t’ {aka ‘unsigned int’} [-Werror=sign-compare]
  677 |     for ( i = 0; i < num_sdevs; i++ )
      |                    ^
xen/lowlevel/xc/xc.c: In function ‘pyxc_physinfo’:
xen/lowlevel/xc/xc.c:988:20: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
  988 |     for ( i = 0; i < sizeof(pinfo.hw_cap)/4; i++ )
      |                    ^
xen/lowlevel/xc/xc.c:994:20: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
  994 |     for ( i = 0; i < ARRAY_SIZE(virtcaps_bits); i++ )
      |                    ^
xen/lowlevel/xc/xc.c:998:24: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
  998 |         for ( i = 0; i < ARRAY_SIZE(virtcaps_bits); i++ )
      |                        ^
xen/lowlevel/xs/xs.c: In function ‘xspy_ls’:
xen/lowlevel/xs/xs.c:191:23: error: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
  191 |         for (i = 0; i < xsval_n; i++)
      |                       ^
xen/lowlevel/xs/xs.c: In function ‘xspy_get_permissions’:
xen/lowlevel/xs/xs.c:297:23: error: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
  297 |         for (i = 0; i < perms_n; i++) {
      |                       ^
cc1: all warnings being treated as errors

Use size_t for loop iterators where it's compared with sizeof() or
similar construct.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/python/xen/lowlevel/xc/xc.c | 13 ++++++++-----
 tools/python/xen/lowlevel/xs/xs.c |  4 ++--
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index 522cbe3b9c..188bfa34da 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -117,7 +117,8 @@ static PyObject *pyxc_domain_create(XcObject *self,
                                     PyObject *kwds)
 {
     uint32_t dom = 0, target = 0;
-    int      ret, i;
+    int      ret;
+    size_t   i;
     PyObject *pyhandle = NULL;
     struct xen_domctl_createdomain config = {
         .handle = {
@@ -295,7 +296,7 @@ static PyObject *pyxc_vcpu_setaffinity(XcObject *self,
 
 static PyObject *pyxc_domain_sethandle(XcObject *self, PyObject *args)
 {
-    int i;
+    size_t i;
     uint32_t dom;
     PyObject *pyhandle;
     xen_domain_handle_t handle;
@@ -336,7 +337,8 @@ static PyObject *pyxc_domain_getinfo(XcObject *self,
     PyObject *list, *info_dict, *pyhandle;
 
     uint32_t first_dom = 0;
-    int max_doms = 1024, nr_doms, i, j;
+    int max_doms = 1024, nr_doms, i;
+    size_t j;
     xc_dominfo_t *info;
 
     static char *kwd_list[] = { "first_dom", "max_doms", NULL };
@@ -631,7 +633,8 @@ static PyObject *pyxc_get_device_group(XcObject *self,
 {
     uint32_t sbdf;
     uint32_t max_sdevs, num_sdevs;
-    int domid, seg, bus, dev, func, rc, i;
+    int domid, seg, bus, dev, func, rc;
+    size_t i;
     PyObject *Pystr;
     char *group_str;
     char dev_str[9];
@@ -971,7 +974,7 @@ static PyObject *pyxc_physinfo(XcObject *self)
 {
     xc_physinfo_t pinfo;
     char cpu_cap[128], virt_caps[128], *p;
-    int i;
+    size_t i;
     const char *virtcap_names[] = { "hvm", "pv" };
     const unsigned virtcaps_bits[] = { XEN_SYSCTL_PHYSCAP_hvm,
                                        XEN_SYSCTL_PHYSCAP_pv };
diff --git a/tools/python/xen/lowlevel/xs/xs.c b/tools/python/xen/lowlevel/xs/xs.c
index 9a0acfc25c..ea50f86bc3 100644
--- a/tools/python/xen/lowlevel/xs/xs.c
+++ b/tools/python/xen/lowlevel/xs/xs.c
@@ -186,7 +186,7 @@ static PyObject *xspy_ls(XsHandle *self, PyObject *args)
     Py_END_ALLOW_THREADS
 
     if (xsval) {
-        int i;
+        size_t i;
         PyObject *val = PyList_New(xsval_n);
         for (i = 0; i < xsval_n; i++)
 #if PY_MAJOR_VERSION >= 3
@@ -276,7 +276,7 @@ static PyObject *xspy_get_permissions(XsHandle *self, PyObject *args)
     struct xs_handle *xh = xshandle(self);
     struct xs_permissions *perms;
     unsigned int perms_n = 0;
-    int i;
+    size_t i;
 
     xs_transaction_t th;
     char *thstr;
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Xen-devel] [PATCH 2/2] tools/tests/cpu-policy: disable -Wformat-overflow
  2019-08-09  2:01 [Xen-devel] [PATCH 1/2] python: fix -Wsign-compare warnings Marek Marczykowski-Górecki
@ 2019-08-09  2:01 ` Marek Marczykowski-Górecki
  2019-08-09  7:51   ` Roger Pau Monné
  2019-08-09 10:41   ` Ian Jackson
  2019-08-09 10:42 ` [Xen-devel] [PATCH 1/2] python: fix -Wsign-compare warnings Ian Jackson
  1 sibling, 2 replies; 10+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-08-09  2:01 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Andrew Cooper, Ian Jackson,
	Marek Marczykowski-Górecki, Jan Beulich,
	Roger Pau Monné

GCC9 complains about "%.12s" format with an argument having exactly 12
bytes and no terminating null character. This is intentional
construct, so disable the warning.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/tests/cpu-policy/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/tests/cpu-policy/Makefile b/tools/tests/cpu-policy/Makefile
index 07dd58f5c2..7d17a4074d 100644
--- a/tools/tests/cpu-policy/Makefile
+++ b/tools/tests/cpu-policy/Makefile
@@ -30,7 +30,7 @@ install: all
 
 .PHONY: uninstall
 
-CFLAGS += -Werror $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -O3
+CFLAGS += -Werror -Wno-format-overflow $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -O3
 CFLAGS += $(APPEND_CFLAGS)
 
 vpath %.c ../../../xen/lib/x86
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Xen-devel] [PATCH 2/2] tools/tests/cpu-policy: disable -Wformat-overflow
  2019-08-09  2:01 ` [Xen-devel] [PATCH 2/2] tools/tests/cpu-policy: disable -Wformat-overflow Marek Marczykowski-Górecki
@ 2019-08-09  7:51   ` Roger Pau Monné
  2019-08-09  8:26     ` Jan Beulich
  2019-08-09 10:41   ` Ian Jackson
  1 sibling, 1 reply; 10+ messages in thread
From: Roger Pau Monné @ 2019-08-09  7:51 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: xen-devel, Ian Jackson, Wei Liu, Jan Beulich, Andrew Cooper

On Fri, Aug 09, 2019 at 04:01:37AM +0200, Marek Marczykowski-Górecki wrote:
> GCC9 complains about "%.12s" format with an argument having exactly 12
> bytes and no terminating null character. This is intentional
> construct, so disable the warning.

IIRC this is deemed as a gcc bug, albeit I'm not sure how we are
supposed to workaround it:

https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01712.html

Disabling the check wholesale seems like a big hammer.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Xen-devel] [PATCH 2/2] tools/tests/cpu-policy: disable -Wformat-overflow
  2019-08-09  7:51   ` Roger Pau Monné
@ 2019-08-09  8:26     ` Jan Beulich
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2019-08-09  8:26 UTC (permalink / raw)
  To: Roger Pau Monné, Marek Marczykowski-Górecki
  Cc: AndrewCooper, Ian Jackson, Wei Liu, xen-devel

On 09.08.2019 09:51, Roger Pau Monné  wrote:
> On Fri, Aug 09, 2019 at 04:01:37AM +0200, Marek Marczykowski-Górecki wrote:
>> GCC9 complains about "%.12s" format with an argument having exactly 12
>> bytes and no terminating null character. This is intentional
>> construct, so disable the warning.
> 
> IIRC this is deemed as a gcc bug, albeit I'm not sure how we are
> supposed to workaround it:
> 
> https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01712.html
> 
> Disabling the check wholesale seems like a big hammer.

Indeed.

I had tried to observe this with a simple small example source
already, and failed. I can't tell whether that's because I've
tried with an almost plain upstream 9.1.0 (and others have some
extra patches in it), or whether that's because there's more to
it than just the array size and format specifier interaction.
Therefore it would help if someone who can actually see the
issue would be able to strip down the full test source here to
a simple reproducer. Depending on whether _that_ then also
fails with plain upstream 9.1.0 the bug would want to be
reported there or to the respective distro(s).

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Xen-devel] [PATCH 2/2] tools/tests/cpu-policy: disable -Wformat-overflow
  2019-08-09  2:01 ` [Xen-devel] [PATCH 2/2] tools/tests/cpu-policy: disable -Wformat-overflow Marek Marczykowski-Górecki
  2019-08-09  7:51   ` Roger Pau Monné
@ 2019-08-09 10:41   ` Ian Jackson
  2019-08-09 10:50     ` Jan Beulich
  1 sibling, 1 reply; 10+ messages in thread
From: Ian Jackson @ 2019-08-09 10:41 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: xen-devel, Roger Pau Monne, Wei Liu, Jan Beulich, Andrew Cooper

Marek Marczykowski-Górecki writes ("[PATCH 2/2] tools/tests/cpu-policy: disable -Wformat-overflow"):
> GCC9 complains about "%.12s" format with an argument having exactly 12
> bytes and no terminating null character. This is intentional
> construct, so disable the warning.

Is there some good reason to have things done this way ?
ISTM that a nicer fix would be to change 12 to 13, earlier.
That way we wouldn't lose justified -Wformat-overflow output.

I appreciate this is only a test program so I'm bikeshedding rather.

As an aside, I hope this code is compiled with -fno-strict-aliasing,
because otherwise it's definitely type-punning in a UB way.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Xen-devel] [PATCH 1/2] python: fix -Wsign-compare warnings
  2019-08-09  2:01 [Xen-devel] [PATCH 1/2] python: fix -Wsign-compare warnings Marek Marczykowski-Górecki
  2019-08-09  2:01 ` [Xen-devel] [PATCH 2/2] tools/tests/cpu-policy: disable -Wformat-overflow Marek Marczykowski-Górecki
@ 2019-08-09 10:42 ` Ian Jackson
  1 sibling, 0 replies; 10+ messages in thread
From: Ian Jackson @ 2019-08-09 10:42 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel, Wei Liu

Marek Marczykowski-Górecki writes ("[PATCH 1/2] python: fix -Wsign-compare warnings"):
> Specifically:
> xen/lowlevel/xc/xc.c: In function ‘pyxc_domain_create’:
> xen/lowlevel/xc/xc.c:147:24: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
>   147 |         for ( i = 0; i < sizeof(xen_domain_handle_t); i++ )

Thanks,

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

and committed.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Xen-devel] [PATCH 2/2] tools/tests/cpu-policy: disable -Wformat-overflow
  2019-08-09 10:41   ` Ian Jackson
@ 2019-08-09 10:50     ` Jan Beulich
  2019-08-09 11:05       ` Ian Jackson
  0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2019-08-09 10:50 UTC (permalink / raw)
  To: Ian Jackson
  Cc: Andrew Cooper, xen-devel, Marek Marczykowski-Górecki,
	Wei Liu, Roger Pau Monne

On 09.08.2019 12:41, Ian Jackson wrote:
> Marek Marczykowski-Górecki writes ("[PATCH 2/2] tools/tests/cpu-policy: disable -Wformat-overflow"):
>> GCC9 complains about "%.12s" format with an argument having exactly 12
>> bytes and no terminating null character. This is intentional
>> construct, so disable the warning.
> 
> Is there some good reason to have things done this way ?
> ISTM that a nicer fix would be to change 12 to 13, earlier.
> That way we wouldn't lose justified -Wformat-overflow output.

Would you mind clarifying which 12 you mean to change to 13?
The one in "%.12s" would, if changed and afaict, then
legitimately trigger the warning. And we've already objected
to the array to get grown.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Xen-devel] [PATCH 2/2] tools/tests/cpu-policy: disable -Wformat-overflow
  2019-08-09 10:50     ` Jan Beulich
@ 2019-08-09 11:05       ` Ian Jackson
  2019-08-09 11:34         ` Jan Beulich
  0 siblings, 1 reply; 10+ messages in thread
From: Ian Jackson @ 2019-08-09 11:05 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Andrew Cooper, xen-devel, Marek Marczykowski-Górecki,
	Wei Liu, Roger Pau Monne

Jan Beulich writes ("Re: [Xen-devel] [PATCH 2/2] tools/tests/cpu-policy: disable -Wformat-overflow"):
> Would you mind clarifying which 12 you mean to change to 13?
> The one in "%.12s" would, if changed and afaict, then
> legitimately trigger the warning. And we've already objected
> to the array to get grown.

I meant the array.  I missed that objection.  I just went and read the
thread
  tests/cpu-policy: fix format-overflow warning by null terminating strings
and it did conclude that the compiler was wrong to complain.

But for me it doesn't follow that the original code is necessarily the
best way of doing things, and I didn't see anyone giving an argument
why simply increasing the array was a bad idea.

C "prefers" null-terminated strings in that they work somewhat better
with a variety of primitives.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Xen-devel] [PATCH 2/2] tools/tests/cpu-policy: disable -Wformat-overflow
  2019-08-09 11:05       ` Ian Jackson
@ 2019-08-09 11:34         ` Jan Beulich
  2019-08-09 12:46           ` Andrew Cooper
  0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2019-08-09 11:34 UTC (permalink / raw)
  To: Ian Jackson
  Cc: Andrew Cooper, xen-devel, Marek Marczykowski-Górecki,
	Wei Liu, Roger Pau Monne

On 09.08.2019 13:05, Ian Jackson wrote:
> Jan Beulich writes ("Re: [Xen-devel] [PATCH 2/2] tools/tests/cpu-policy: disable -Wformat-overflow"):
>> Would you mind clarifying which 12 you mean to change to 13?
>> The one in "%.12s" would, if changed and afaict, then
>> legitimately trigger the warning. And we've already objected
>> to the array to get grown.
> 
> I meant the array.  I missed that objection.  I just went and read the
> thread
>    tests/cpu-policy: fix format-overflow warning by null terminating strings
> and it did conclude that the compiler was wrong to complain.
> 
> But for me it doesn't follow that the original code is necessarily the
> best way of doing things, and I didn't see anyone giving an argument
> why simply increasing the array was a bad idea.
> 
> C "prefers" null-terminated strings in that they work somewhat better
> with a variety of primitives.

Right, but the %.<num>s specification exits precisely to allow
to deal with potentially not nul-terminated strings. ACPI code
in the hypervisor makes quite a bit of use of this, for example,
without triggering any compiler warnings with 9.1.0.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Xen-devel] [PATCH 2/2] tools/tests/cpu-policy: disable -Wformat-overflow
  2019-08-09 11:34         ` Jan Beulich
@ 2019-08-09 12:46           ` Andrew Cooper
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Cooper @ 2019-08-09 12:46 UTC (permalink / raw)
  To: Jan Beulich, Ian Jackson
  Cc: xen-devel, Marek Marczykowski-Górecki, Wei Liu, Roger Pau Monne

On 09/08/2019 12:34, Jan Beulich wrote:
> On 09.08.2019 13:05, Ian Jackson wrote:
>> Jan Beulich writes ("Re: [Xen-devel] [PATCH 2/2]
>> tools/tests/cpu-policy: disable -Wformat-overflow"):
>>> Would you mind clarifying which 12 you mean to change to 13?
>>> The one in "%.12s" would, if changed and afaict, then
>>> legitimately trigger the warning. And we've already objected
>>> to the array to get grown.
>>
>> I meant the array.  I missed that objection.  I just went and read the
>> thread
>>    tests/cpu-policy: fix format-overflow warning by null terminating
>> strings
>> and it did conclude that the compiler was wrong to complain.
>>
>> But for me it doesn't follow that the original code is necessarily the
>> best way of doing things, and I didn't see anyone giving an argument
>> why simply increasing the array was a bad idea.
>>
>> C "prefers" null-terminated strings in that they work somewhat better
>> with a variety of primitives.
>
> Right, but the %.<num>s specification exits precisely to allow
> to deal with potentially not nul-terminated strings. ACPI code
> in the hypervisor makes quite a bit of use of this, for example,
> without triggering any compiler warnings with 9.1.0.

I also wonder whether
https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-nonstring-variable-attribute
might be a way of fixing this, seeing as it exists specifically for the
purpose.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-08-09 12:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-09  2:01 [Xen-devel] [PATCH 1/2] python: fix -Wsign-compare warnings Marek Marczykowski-Górecki
2019-08-09  2:01 ` [Xen-devel] [PATCH 2/2] tools/tests/cpu-policy: disable -Wformat-overflow Marek Marczykowski-Górecki
2019-08-09  7:51   ` Roger Pau Monné
2019-08-09  8:26     ` Jan Beulich
2019-08-09 10:41   ` Ian Jackson
2019-08-09 10:50     ` Jan Beulich
2019-08-09 11:05       ` Ian Jackson
2019-08-09 11:34         ` Jan Beulich
2019-08-09 12:46           ` Andrew Cooper
2019-08-09 10:42 ` [Xen-devel] [PATCH 1/2] python: fix -Wsign-compare warnings Ian Jackson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).