[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] xen/device_tree: silence ambiguous integer casting warning error



Hi,

On 19/04/2022 16:46, Paran Lee wrote:
GCC with "-g -Wall -Wextra" option throws warning message as below:

s/warning/error/


error: comparison of integer expressions of different signedness:
  ‘int’ and ‘unsigned int’ [-Werror=sign-compare]

Can you post the full log?


Silence the warning by correcting the integer type.

IIRC most of the code you touch below is from Linux and had very limited changes afterwards. So while I agree the changes are good, AFAIK they are not (latent?) bugs and we don't build by default with -Wextra.

Therefore, I would prefer if they are upstreamed to Linux first and then backported to Xen.


Signed-off-by: Paran Lee <p4ranlee@xxxxxxxxx>
---
  xen/common/device_tree.c | 31 +++++++++++++++----------------
  1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 4aae281e89..402e465c7a 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -79,7 +79,7 @@ struct dt_bus
      const char *addresses;
      bool_t (*match)(const struct dt_device_node *node);
      void (*count_cells)(const struct dt_device_node *child,
-                        int *addrc, int *sizec);
+                        unsigned int *addrc, unsigned int *sizec);
      u64 (*map)(__be32 *addr, const __be32 *range, int na, int ns, int pna);
      int (*translate)(__be32 *addr, u64 offset, int na);
      unsigned int (*get_flags)(const __be32 *addr);
@@ -569,7 +569,7 @@ static bool_t dt_bus_default_match(const struct 
dt_device_node *node)
  }
static void dt_bus_default_count_cells(const struct dt_device_node *dev,
-                                int *addrc, int *sizec)
+                                       unsigned int *addrc, unsigned int 
*sizec)
  {
      if ( addrc )
          *addrc = dt_n_addr_cells(dev);

Technically, the prototype of dt_n_addr_cells() is "int ... (". So now you are casting an "int" to an "unsigned int". AFAICT, dt_n_addr_cells() will never return a negative value. So we should propage the "unsigned int".

@@ -649,7 +649,7 @@ static bool_t dt_bus_pci_match(const struct dt_device_node 
*np)
  }
static void dt_bus_pci_count_cells(const struct dt_device_node *np,
-                                  int *addrc, int *sizec)
+                                  unsigned int *addrc, unsigned int *sizec)
  {
      if (addrc)
          *addrc = 3;
@@ -737,7 +737,7 @@ static const struct dt_bus dt_busses[] =
static const struct dt_bus *dt_match_bus(const struct dt_device_node *np)
  {
-    int i;
+    long unsigned int i;

This should be size_t.

for ( i = 0; i < ARRAY_SIZE(dt_busses); i++ )
          if ( !dt_busses[i].match || dt_busses[i].match(np) )
@@ -754,7 +754,8 @@ static const __be32 *dt_get_address(const struct 
dt_device_node *dev,
      u32 psize;
      const struct dt_device_node *parent;
      const struct dt_bus *bus;
-    int onesize, i, na, ns;
+    unsigned int i;
+    unsigned int onesize, na, ns; >
      /* Get parent & match bus type */
      parent = dt_get_parent(dev);
@@ -797,8 +798,7 @@ static int dt_translate_one(const struct dt_device_node 
*parent,
                              int pna, const char *rprop)
  {
      const __be32 *ranges;
-    unsigned int rlen;
-    int rone;
+    unsigned int rlen, rone;
      u64 offset = DT_BAD_ADDR;
ranges = dt_get_property(parent, rprop, &rlen);
@@ -857,7 +857,7 @@ static u64 __dt_translate_address(const struct 
dt_device_node *dev,
      const struct dt_device_node *parent = NULL;
      const struct dt_bus *bus, *pbus;
      __be32 addr[DT_MAX_ADDR_CELLS];
-    int na, ns, pna, pns;
+    unsigned int na, ns, pna, pns;
      u64 result = DT_BAD_ADDR;
dt_dprintk("DT: ** translation for device %s **\n", dev->full_name);
@@ -966,8 +966,7 @@ int dt_for_each_range(const struct dt_device_node *dev,
      const struct dt_bus *bus, *pbus;
      const __be32 *ranges;
      __be32 addr[DT_MAX_ADDR_CELLS];
-    unsigned int rlen;
-    int na, ns, pna, pns, rone;
+    unsigned int rlen, na, ns, pna, pns, rone;
bus = dt_match_bus(dev);
      if ( !bus )
@@ -1134,7 +1133,7 @@ unsigned int dt_number_of_address(const struct 
dt_device_node *dev)
      u32 psize;
      const struct dt_device_node *parent;
      const struct dt_bus *bus;
-    int onesize, na, ns;
+    unsigned int onesize, na, ns;
/* Get parent & match bus type */
      parent = dt_get_parent(dev);
@@ -1169,8 +1168,8 @@ int dt_for_each_irq_map(const struct dt_device_node *dev,
      const struct dt_device_node *ipar, *tnode, *old = NULL;
      const __be32 *tmp, *imap;
      u32 intsize = 1, addrsize, pintsize = 0, paddrsize = 0;
-    u32 imaplen;
-    int i, ret;
+    u32 i, imaplen;
+    int ret;
struct dt_raw_irq dt_raw_irq;
      struct dt_irq dt_irq;
@@ -1354,8 +1353,8 @@ static int dt_irq_map_raw(const struct dt_device_node 
*parent,
      const struct dt_device_node *ipar, *tnode, *old = NULL, *newpar = NULL;
      const __be32 *tmp, *imap, *imask;
      u32 intsize = 1, addrsize, newintsize = 0, newaddrsize = 0;
-    u32 imaplen;
-    int match, i;
+    u32 i, imaplen;
+    int match;
dt_dprintk("dt_irq_map_raw: par=%s,intspec=[0x%08x 0x%08x...],ointsize=%d\n",
                 parent->full_name, be32_to_cpup(intspec),
@@ -1737,7 +1736,7 @@ static int __dt_parse_phandle_with_args(const struct 
dt_device_node *np,
if ( out_args )
              {
-                int i;
+                u32 i;
WARN_ON(count > MAX_PHANDLE_ARGS);
                  if (count > MAX_PHANDLE_ARGS)

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.