linux-kernel - Re: [PATCH v2] dma: vdma: Fix compilation warnings

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] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 31 Mar 2015 10:27:59 +0530
From:	Vinod Koul <vinod.koul@...el.com>
To:	Stephen Rothwell <sfr@...b.auug.org.au>
Cc:	Kedareswara rao Appana <appana.durga.rao@...inx.com>,
	linux-next@...r.kernel.org, linux-kernel@...r.kernel.org,
	anirudh@...inx.com, Kedareswara rao Appana <appanad@...inx.com>
Subject: Re: [PATCH v2] dma: vdma: Fix compilation warnings

On Tue, Mar 31, 2015 at 07:35:43AM +1100, Stephen Rothwell wrote:
Hi Stephen,
>
> On Mon, 30 Mar 2015 22:48:46 +0530 Vinod Koul <vinod.koul@...el.com> wrote:
> >
> > On Mon, Mar 30, 2015 at 06:48:29PM +0530, Kedareswara rao Appana wrote:
> > > This patch fixes the following compilation warnings.
> > > In file included from drivers/dma/xilinx/xilinx_vdma.c:26:0:
> > > include/linux/dmapool.h:18:4: warning: 'struct device' declared inside parameter list
> > >     size_t size, size_t align, size_t allocation);
> > >     ^
> > > include/linux/dmapool.h:18:4: warning: its scope is only this definition or declaration, which is probably not what you want
> > > include/linux/dmapool.h:31:7: warning: 'struct device' declared inside parameter list
> > >        size_t size, size_t align, size_t allocation);
> > >        ^
> > > drivers/dma/xilinx/xilinx_vdma.c: In function 'xilinx_vdma_alloc_chan_resources':
> > > drivers/dma/xilinx/xilinx_vdma.c:501:20: warning: passing argument 2 of 'dma_pool_create' from incompatible pointer type
> > >   chan->desc_pool = dma_pool_create("xilinx_vdma_desc_pool",
> > >                     ^
> > > In file included from drivers/dma/xilinx/xilinx_vdma.c:26:0:
> > > include/linux/dmapool.h:17:18: note: expected 'struct device *' but argument is of type 'struct device *'
> > >  struct dma_pool *dma_pool_create(const char *name, struct device *dev, .
> > > 
> > Well this does fix this error but this can also be fixed by rearranging the
> > driver header files order.  Since I am not inclined to update a patch for
> > dmapool.h I would go for rearranging drivers header
> > 
> > --><8---------------><8--------------
> > 
> > diff --git a/drivers/dma/xilinx/xilinx_vdma.c
> > b/drivers/dma/xilinx/xilinx_vdma.c
> > index d8434d465885..356ca4bc0ea5 100644
> > --- a/drivers/dma/xilinx/xilinx_vdma.c
> > +++ b/drivers/dma/xilinx/xilinx_vdma.c
> > @@ -23,12 +23,12 @@
> >   */
> > 
> >  #include <linux/bitops.h>
> > -#include <linux/dmapool.h>
> >  #include <linux/dma/xilinx_dma.h>
> >  #include <linux/init.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/io.h>
> >  #include <linux/module.h>
> > +#include <linux/dmapool.h>
> >  #include <linux/of_address.h>
> >  #include <linux/of_dma.h>
> >  #include <linux/of_platform.h>
> > 
> > Any objections?
> 
> Yes.  The error is in dmapool.h so it should be fixed once and for
> all.  The supplied patch is very unintrusive and means that the problem
> won't reappear when someone does some rearrangement of includes in the
> future.  The file in question really has no particular maintainer.
> Even after your suggested patch, dmapool.h still depend on an implicit
> include of device.h.
I agree with your points, but isnt the order of headers also a thumb rule.
Typicaly a driver file will include core includes followed by subsystem
specfic includes.

Should a header have no dependency for its include ? I do come across
multiple examples of this in kernel

Yes fixing it in dmapool is also correct, so should we move to having
headers agnostic to the order of inclusion eventually ?

Thanks
-- 
~Vinod

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)