[PATCH 1/1] xen-netback: remove compilation warning

[PATCH 1/1] xen-netback: remove compilation warning

[pedro]

From: pmarzo <marzo.pedro@gmail.com>

offset and size are of type uint16_t so the %lu gives a warning
A %u specifier, the same used in size makes gcc happy
Not sure if a %x would be more correct

Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com>
---
 drivers/net/xen-netback/netback.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index f7a31d2..3888a2b 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1248,7 +1248,7 @@ static void xenvif_tx_build_gops(struct xenvif_queue *queue,
 		/* No crossing a page as the payload mustn't fragment. */
 		if (unlikely((txreq.offset + txreq.size) > PAGE_SIZE)) {
 			netdev_err(queue->vif->dev,
-				   "txreq.offset: %x, size: %u, end: %lu\n",
+				   "txreq.offset: %x, size: %u, end: %u\n",
 				   txreq.offset, txreq.size,
 				   (txreq.offset&~PAGE_MASK) + txreq.size);
 			xenvif_fatal_tx_err(queue->vif);
-- 
1.9.1

Re: [PATCH 1/1] xen-netback: remove compilation warning

[Wei Liu]

On Thu, Feb 26, 2015 at 09:25:41AM +0100, pedro wrote:
> From: pmarzo <marzo.pedro@gmail.com>
> 
> offset and size are of type uint16_t so the %lu gives a warning
> A %u specifier, the same used in size makes gcc happy
> Not sure if a %x would be more correct
> 
> Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

Thanks

> ---
>  drivers/net/xen-netback/netback.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index f7a31d2..3888a2b 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -1248,7 +1248,7 @@ static void xenvif_tx_build_gops(struct xenvif_queue *queue,
>  		/* No crossing a page as the payload mustn't fragment. */
>  		if (unlikely((txreq.offset + txreq.size) > PAGE_SIZE)) {
>  			netdev_err(queue->vif->dev,
> -				   "txreq.offset: %x, size: %u, end: %lu\n",
> +				   "txreq.offset: %x, size: %u, end: %u\n",
>  				   txreq.offset, txreq.size,
>  				   (txreq.offset&~PAGE_MASK) + txreq.size);
>  			xenvif_fatal_tx_err(queue->vif);
> -- 
> 1.9.1

Re: [PATCH 1/1] xen-netback: remove compilation warning

[David Miller]

From: pedro <marzo.pedro@gmail.com>
Date: Thu, 26 Feb 2015 09:25:41 +0100

> From: pmarzo <marzo.pedro@gmail.com>
> 
> offset and size are of type uint16_t so the %lu gives a warning
> A %u specifier, the same used in size makes gcc happy
> Not sure if a %x would be more correct
> 
> Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com>

This patch actually adds a warning on my machine, and your analysis
of the types is therefore probably incorrect:

drivers/net/xen-netback/netback.c: In function ‘xenvif_tx_build_gops’:
drivers/net/xen-netback/netback.c:1259:8: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 5 has type ‘long unsigned int’ [-Wformat=]

The issue is probably "~PAGE_MASK" and I think the type of that
propagates into the type of the overall calculation.

Re: [PATCH 1/1] xen-netback: remove compilation warning

[pmarzo]

On jue, 2015-02-26 at 11:30 -0500, David Miller wrote:
> From: pedro <marzo.pedro@gmail.com>
> Date: Thu, 26 Feb 2015 09:25:41 +0100
> 
> > From: pmarzo <marzo.pedro@gmail.com>
> > 
> > offset and size are of type uint16_t so the %lu gives a warning
> > A %u specifier, the same used in size makes gcc happy
> > Not sure if a %x would be more correct
> > 
> > Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com>
> 
> This patch actually adds a warning on my machine, and your analysis
> of the types is therefore probably incorrect:
> 
> drivers/net/xen-netback/netback.c: In function ‘xenvif_tx_build_gops’:
> drivers/net/xen-netback/netback.c:1259:8: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 5 has type ‘long unsigned int’ [-Wformat=]

You are right, this patch is completely wrong for i386, it gives me a
warning too. I should have checked that before, sorry. 

I should also have said I am using a cross compiler, which is the one
that gives the warning compiling the current code:
arm-linux-gnueabi-gcc --version
arm-linux-gnueabi-gcc (Ubuntu/Linaro 4.7.3-12ubuntu1) 4.7.3


> 
> The issue is probably "~PAGE_MASK" and I think the type of that
> propagates into the type of the overall calculation.
That is what is probably happening, operations must be done to operands
of the same size, and the intel compiler is casting everything to
unsigned long (because I have a 64 bit machine??), but the arm compiler
is casting to unsigned int :-(

PAGE_MASK is defined as a number without any cast, so not sure which
compiler is right
	#define PAGE_SHIFT              12
	#define PAGE_MASK               (~((1 << PAGE_SHIFT) - 1))

This new patch fixes the warning for the arm gcc compiler and the i386
compiler, it just makes sure everything is cast to unsigned long
Could you please forget the previous one and give your opinion about
this one?

--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1248,9 +1248,10 @@ static void xenvif_tx_build_gops(struct
xenvif_queue *queue,
                /* No crossing a page as the payload mustn't fragment.
*/
                if (unlikely((txreq.offset + txreq.size) > PAGE_SIZE)) {
                        netdev_err(queue->vif->dev,
-                                  "txreq.offset: %x, size: %u, end: %u
\n",
+                                  "txreq.offset: %x, size: %u, end: %lu
\n",
                                   txreq.offset, txreq.size,
-                                  (txreq.offset&~PAGE_MASK) +
txreq.size);
+                                  ((unsigned
long)txreq.offset&~PAGE_MASK)
+                                        + txreq.size);
                        xenvif_fatal_tx_err(queue->vif);
                        break;
                }
-- 
1.9.1