On patch "device.h: audit and cleanup users in main include dir"

On patch "device.h: audit and cleanup users in main include dir"

[Stefan Richter]

Hi Paul,

commit 313162d0b83836e2f57e51b9b8650fb4b9c396ea is not 100.00% correct.

--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -2,7 +2,6 @@
 #define _LINUX_FIREWIRE_H
 
 #include <linux/completion.h>
-#include <linux/device.h>
 #include <linux/dma-mapping.h>
 #include <linux/kernel.h>
 #include <linux/kref.h>
@@ -68,6 +67,8 @@
 #define CSR_MODEL              0x17
 #define CSR_DIRECTORY_ID       0x20
 
+struct device;
+
 struct fw_csr_iterator {
        const u32 *p;
        const u32 *end;

But struct fw_device and struct fw_unit, defined in include/firewire.h,
incorporate struct device.

Furthermore, the inline function definitions
    fw_device,
    fw_unit,
    fw_unit_get,
    fw_unit_put,
    fw_parent_device
all require definitions from linux/device.h.
-- 
Stefan Richter
-=====-===-- --== ==---
http://arcgraph.de/sr/

Re: On patch "device.h: audit and cleanup users in main include dir"

[Paul Gortmaker]

[On patch "device.h: audit and cleanup users in main include dir"] On 24/03/2012 (Sat 21:21) Stefan Richter wrote:

> Hi Paul,
> 
> commit 313162d0b83836e2f57e51b9b8650fb4b9c396ea is not 100.00% correct.
> 
> --- a/include/linux/firewire.h
> +++ b/include/linux/firewire.h
> @@ -2,7 +2,6 @@
>  #define _LINUX_FIREWIRE_H
>  
>  #include <linux/completion.h>
> -#include <linux/device.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/kernel.h>
>  #include <linux/kref.h>
> @@ -68,6 +67,8 @@
>  #define CSR_MODEL              0x17
>  #define CSR_DIRECTORY_ID       0x20
>  
> +struct device;
> +
>  struct fw_csr_iterator {
>         const u32 *p;
>         const u32 *end;
> 
> But struct fw_device and struct fw_unit, defined in include/firewire.h,
> incorporate struct device.

Hi Stefan,

You are correct.  I did read them all and check instances of device, so
I'm not sure how I misread this file.  Did you have a specific build
that triggered a failure, or you just happened to spot it?  I'm guessing
that it gets device.h by another path and hence won't show up in build
coverage.

If so I'll definitely get it fixed, but I might wait a day or two to see
if anything else pops up, if it isn't explicitly causing any breakage.

Thanks for the report,
Paul.
--

> 
> Furthermore, the inline function definitions
>     fw_device,
>     fw_unit,
>     fw_unit_get,
>     fw_unit_put,
>     fw_parent_device
> all require definitions from linux/device.h.
> -- 
> Stefan Richter
> -=====-===-- --== ==---
> http://arcgraph.de/sr/

Re: On patch "device.h: audit and cleanup users in main include dir"

[Stefan Richter]

On Mar 24 Paul Gortmaker wrote:
> [On patch "device.h: audit and cleanup users in main include dir"] On 24/03/2012 (Sat 21:21) Stefan Richter wrote:
[...]
> > struct fw_device and struct fw_unit, defined in include/firewire.h,
> > incorporate struct device.
> 
> Hi Stefan,
> 
> You are correct.  I did read them all and check instances of device, so
> I'm not sure how I misread this file.  Did you have a specific build
> that triggered a failure, or you just happened to spot it? I'm guessing
> that it gets device.h by another path and hence won't show up in build
> coverage.

I just saw the commit coming in; there is no build failure.  (I should have
test-built before posting.)  All files which include linux/firewire.h
evidently include linux/device.h before it.

> If so I'll definitely get it fixed, but I might wait a day or two to see
> if anything else pops up, if it isn't explicitly causing any breakage.

If you don't receive any issues with the rest of your tree-wide commit,
just do nothing and I will add a trivial firewire.h correction myself for
the next merge window.

> > Furthermore, the inline function definitions
> >     fw_device,
> >     fw_unit,
> >     fw_unit_get,
> >     fw_unit_put,
> >     fw_parent_device
> > all require definitions from linux/device.h.

Correcting myself:  Only the the latter three of these use linux/device.h.
-- 
Stefan Richter
-=====-===-- --== ==--=
http://arcgraph.de/sr/