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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1425033952.3659.23.camel@CEM14014B>
Date:	Fri, 27 Feb 2015 11:45:52 +0100
From:	pmarzo <marzo.pedro@...il.com>
To:	David Miller <davem@...emloft.net>
Cc:	ian.campbell@...rix.com, wei.liu2@...rix.com,
	xen-devel@...ts.xenproject.org, netdev@...r.kernel.org
Subject: Re: [PATCH 1/1] xen-netback: remove compilation warning


On jue, 2015-02-26 at 11:30 -0500, David Miller wrote:
> From: pedro <marzo.pedro@...il.com>
> Date: Thu, 26 Feb 2015 09:25:41 +0100
> 
> > From: pmarzo <marzo.pedro@...il.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@...il.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




    



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html