[PATCH -next] bcma: main.c needs to include <linux/slab.h>

[PATCH -next] bcma: main.c needs to include <linux/slab.h>

[Geert Uytterhoeven]

m68k allmodconfig:

drivers/bcma/main.c: In function ‘bcma_release_core_dev’:
drivers/bcma/main.c:68: error: implicit declaration of function ‘kfree’

Signed-off-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
--
http://kisskb.ellerman.id.au/kisskb/buildresult/4243344/

 drivers/bcma/main.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c
index ba15105..08a14a3 100644
--- a/drivers/bcma/main.c
+++ b/drivers/bcma/main.c
@@ -7,6 +7,7 @@
 
 #include "bcma_private.h"
 #include <linux/bcma/bcma.h>
+#include <linux/slab.h>
 
 MODULE_DESCRIPTION("Broadcom's specific AMBA driver");
 MODULE_LICENSE("GPL");
-- 
1.7.0.4

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -next] bcma: main.c needs to include <linux/slab.h>

[Rafał Miłecki]

2011/6/26 Geert Uytterhoeven <geert@linux-m68k.org>:
> m68k allmodconfig:
>
> drivers/bcma/main.c: In function ‘bcma_release_core_dev’:
> drivers/bcma/main.c:68: error: implicit declaration of function ‘kfree’

We already include slab.h in:
host_pci.c
scan.c
sprom.c

Maybe we can just include this in bcma.h as a better solution? We
could drop other includes then.
Can you submit patch for this?

-- 
Rafał

Re: [PATCH -next] bcma: main.c needs to include <linux/slab.h>

[Geert Uytterhoeven]

2011/6/27 Rafał Miłecki <zajec5@gmail.com>:
> 2011/6/26 Geert Uytterhoeven <geert@linux-m68k.org>:
>> m68k allmodconfig:
>>
>> drivers/bcma/main.c: In function ‘bcma_release_core_dev’:
>> drivers/bcma/main.c:68: error: implicit declaration of function ‘kfree’
>
> We already include slab.h in:
> host_pci.c
> scan.c
> sprom.c
>
> Maybe we can just include this in bcma.h as a better solution? We
> could drop other includes then.
> Can you submit patch for this?

If bcma.h doesn't use linux/slab.h, IMHO it should not include linux/slab.h.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH -next] bcma: main.c needs to include <linux/slab.h>

[Rafał Miłecki]

W dniu 27 czerwca 2011 16:09 użytkownik Geert Uytterhoeven
<geert@linux-m68k.org> napisał:
> 2011/6/27 Rafał Miłecki <zajec5@gmail.com>:
>> 2011/6/26 Geert Uytterhoeven <geert@linux-m68k.org>:
>>> m68k allmodconfig:
>>>
>>> drivers/bcma/main.c: In function ‘bcma_release_core_dev’:
>>> drivers/bcma/main.c:68: error: implicit declaration of function ‘kfree’
>>
>> We already include slab.h in:
>> host_pci.c
>> scan.c
>> sprom.c
>>
>> Maybe we can just include this in bcma.h as a better solution? We
>> could drop other includes then.
>> Can you submit patch for this?
>
> If bcma.h doesn't use linux/slab.h, IMHO it should not include linux/slab.h.

I guess you want to avoid auto(chain)-including linux/slab.h by bcma
drivers? OK, just use bcma_private.h instead.

-- 
Rafał

Re: [PATCH -next] bcma: main.c needs to include <linux/slab.h>

[Alexey Dobriyan]

2011/6/27 Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> 2011/6/26 Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>:
>> m68k allmodconfig:
>>
>> drivers/bcma/main.c: In function ‘bcma_release_core_dev’:
>> drivers/bcma/main.c:68: error: implicit declaration of function ‘kfree’
>
> We already include slab.h in:
> host_pci.c
> scan.c
> sprom.c
>
> Maybe we can just include this in bcma.h as a better solution?

It isn't better solution.
It results in situation where unnecessary inclusion will be done.
Maybe it's not the case now, but it will be in future.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -next] bcma: main.c needs to include <linux/slab.h>

[Rafał Miłecki]

W dniu 27 czerwca 2011 16:24 użytkownik Alexey Dobriyan
<adobriyan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> napisał:
> 2011/6/27 Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>> 2011/6/26 Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>:
>>> m68k allmodconfig:
>>>
>>> drivers/bcma/main.c: In function ‘bcma_release_core_dev’:
>>> drivers/bcma/main.c:68: error: implicit declaration of function ‘kfree’
>>
>> We already include slab.h in:
>> host_pci.c
>> scan.c
>> sprom.c
>>
>> Maybe we can just include this in bcma.h as a better solution?
>
> It isn't better solution.
> It results in situation where unnecessary inclusion will be done.
> Maybe it's not the case now, but it will be in future.

Scanning code is required for every BCMA board, so we already include
linux/slab.h on every configuration. No matter if this is PCI host
board, or SoC, or whatever we will support in the future.
Now we discovered this is also needed in main.c, which will be always compiled.

That's why I think it's safe to include linux/slab.h in bcma_private.h.
But if that's just my opinion, everybody think it's wrong idea, I'm OK with it.

-- 
Rafał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -next] bcma: main.c needs to include <linux/slab.h>

[Pavel Roskin]

On 06/27/2011 10:24 AM, Alexey Dobriyan wrote:
> 2011/6/27 Rafał Miłecki<zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>> 2011/6/26 Geert Uytterhoeven<geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>:
>>> m68k allmodconfig:
>>>
>>> drivers/bcma/main.c: In function ‘bcma_release_core_dev’:
>>> drivers/bcma/main.c:68: error: implicit declaration of function ‘kfree’
>>
>> We already include slab.h in:
>> host_pci.c
>> scan.c
>> sprom.c
>>
>> Maybe we can just include this in bcma.h as a better solution?
>
> It isn't better solution.
> It results in situation where unnecessary inclusion will be done.
> Maybe it's not the case now, but it will be in future.

I agree.  kfree() is used in main.c, not in bcma.h.  There is no need 
for all files that include bcma.h to include linux/slab.h, especially 
(but not only) because bcma.h is not a private header.

-- 
Regards,
Pavel Roskin
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -next] bcma: main.c needs to include <linux/slab.h>

[Rafał Miłecki]

2011/6/27 Pavel Roskin <proski-mXXj517/zsQ@public.gmane.org>:
> On 06/27/2011 10:24 AM, Alexey Dobriyan wrote:
>>
>> 2011/6/27 Rafał Miłecki<zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>>>
>>> 2011/6/26 Geert Uytterhoeven<geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>:
>>>>
>>>> m68k allmodconfig:
>>>>
>>>> drivers/bcma/main.c: In function ‘bcma_release_core_dev’:
>>>> drivers/bcma/main.c:68: error: implicit declaration of function ‘kfree’
>>>
>>> We already include slab.h in:
>>> host_pci.c
>>> scan.c
>>> sprom.c
>>>
>>> Maybe we can just include this in bcma.h as a better solution?
>>
>> It isn't better solution.
>> It results in situation where unnecessary inclusion will be done.
>> Maybe it's not the case now, but it will be in future.
>
> I agree.  kfree() is used in main.c, not in bcma.h.  There is no need for
> all files that include bcma.h to include linux/slab.h, especially (but not
> only) because bcma.h is not a private header.

You ignore the fact I clarified my idea to use bcma_private.h instead of bcma.h.

-- 
Rafał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -next] bcma: main.c needs to include <linux/slab.h>

[Geert Uytterhoeven]

2011/6/27 Rafał Miłecki <zajec5@gmail.com>:
> 2011/6/27 Pavel Roskin <proski@gnu.org>:
>> On 06/27/2011 10:24 AM, Alexey Dobriyan wrote:
>>> 2011/6/27 Rafał Miłecki<zajec5@gmail.com>:
>>>> 2011/6/26 Geert Uytterhoeven<geert@linux-m68k.org>:
>>>>>
>>>>> m68k allmodconfig:
>>>>>
>>>>> drivers/bcma/main.c: In function ‘bcma_release_core_dev’:
>>>>> drivers/bcma/main.c:68: error: implicit declaration of function ‘kfree’
>>>>
>>>> We already include slab.h in:
>>>> host_pci.c
>>>> scan.c
>>>> sprom.c
>>>>
>>>> Maybe we can just include this in bcma.h as a better solution?
>>>
>>> It isn't better solution.
>>> It results in situation where unnecessary inclusion will be done.
>>> Maybe it's not the case now, but it will be in future.
>>
>> I agree.  kfree() is used in main.c, not in bcma.h.  There is no need for
>> all files that include bcma.h to include linux/slab.h, especially (but not
>> only) because bcma.h is not a private header.
>
> You ignore the fact I clarified my idea to use bcma_private.h instead of bcma.h.

One day A Cleaner will remove it again, seeing bcma_private.h doesn't
use any slab
interface, and it still seems to compile on his platform of choice
(which implicitly
pulls in slab.h).

If it's put in main.c, The Cleaner will notice main.c uses kfree(),
and won't touch it.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH -next] bcma: main.c needs to include <linux/slab.h>

[Rafał Miłecki]

W dniu 27 czerwca 2011 16:53 użytkownik Geert Uytterhoeven
<geert@linux-m68k.org> napisał:
> 2011/6/27 Rafał Miłecki <zajec5@gmail.com>:
>> 2011/6/27 Pavel Roskin <proski@gnu.org>:
>>> On 06/27/2011 10:24 AM, Alexey Dobriyan wrote:
>>>> 2011/6/27 Rafał Miłecki<zajec5@gmail.com>:
>>>>> 2011/6/26 Geert Uytterhoeven<geert@linux-m68k.org>:
>>>>>>
>>>>>> m68k allmodconfig:
>>>>>>
>>>>>> drivers/bcma/main.c: In function ‘bcma_release_core_dev’:
>>>>>> drivers/bcma/main.c:68: error: implicit declaration of function ‘kfree’
>>>>>
>>>>> We already include slab.h in:
>>>>> host_pci.c
>>>>> scan.c
>>>>> sprom.c
>>>>>
>>>>> Maybe we can just include this in bcma.h as a better solution?
>>>>
>>>> It isn't better solution.
>>>> It results in situation where unnecessary inclusion will be done.
>>>> Maybe it's not the case now, but it will be in future.
>>>
>>> I agree.  kfree() is used in main.c, not in bcma.h.  There is no need for
>>> all files that include bcma.h to include linux/slab.h, especially (but not
>>> only) because bcma.h is not a private header.
>>
>> You ignore the fact I clarified my idea to use bcma_private.h instead of bcma.h.
>
> One day A Cleaner will remove it again, seeing bcma_private.h doesn't
> use any slab
> interface, and it still seems to compile on his platform of choice
> (which implicitly
> pulls in slab.h).
>
> If it's put in main.c, The Cleaner will notice main.c uses kfree(),
> and won't touch it.

A Cleaner should review all files that use bcma_private.h and notice kfree() ;)

But as I said, I don't really argue.

John, if that's OK for you, please take it.

-- 
Rafał

Re: [PATCH -next] bcma: main.c needs to include <linux/slab.h>

[Arend van Spriel]

On 06/27/2011 04:43 PM, Rafał Miłecki wrote:
> W dniu 27 czerwca 2011 16:24 użytkownik Alexey Dobriyan
> <adobriyan@gmail.com>  napisał:
>> 2011/6/27 Rafał Miłecki<zajec5@gmail.com>:
>>> 2011/6/26 Geert Uytterhoeven<geert@linux-m68k.org>:
>>>> m68k allmodconfig:
>>>>
>>>> drivers/bcma/main.c: In function ‘bcma_release_core_dev’:
>>>> drivers/bcma/main.c:68: error: implicit declaration of function ‘kfree’
>>> We already include slab.h in:
>>> host_pci.c
>>> scan.c
>>> sprom.c
>>>
>>> Maybe we can just include this in bcma.h as a better solution?
>> It isn't better solution.
>> It results in situation where unnecessary inclusion will be done.
>> Maybe it's not the case now, but it will be in future.
> Scanning code is required for every BCMA board, so we already include
> linux/slab.h on every configuration. No matter if this is PCI host
> board, or SoC, or whatever we will support in the future.
> Now we discovered this is also needed in main.c, which will be always compiled.
>
> That's why I think it's safe to include linux/slab.h in bcma_private.h.
> But if that's just my opinion, everybody think it's wrong idea, I'm OK with it.

My rule of thumb is: Header file a.h may only include header b.h when 
a.h needs some definition from b.h. Convenience is never a good reason 
for nested includes.

Gr. AvS

-- 
Almost nobody dances sober, unless they happen to be insane.
-- H.P. Lovecraft --

Re: [PATCH -next] bcma: main.c needs to include <linux/slab.h>

[Johannes Berg]

On Mon, 2011-06-27 at 18:49 +0200, Arend van Spriel wrote:

> > That's why I think it's safe to include linux/slab.h in bcma_private.h.
> > But if that's just my opinion, everybody think it's wrong idea, I'm OK with it.
> 
> My rule of thumb is: Header file a.h may only include header b.h when 
> a.h needs some definition from b.h. Convenience is never a good reason 
> for nested includes.

Yeah, good rule. Consider if you have a.h, b.h and z.c, z.c needs b.h
but not a.h, and now b.h includes a.h ("for convenience") -- changing
a.h would needlessly recompile z.c. Now, changing slab.h will probably
recompile everything anyway, but still...

johannes

Re: [PATCH -next] bcma: main.c needs to include <linux/slab.h>

[Pavel Roskin]

On 06/27/2011 12:57 PM, Johannes Berg wrote:

> Yeah, good rule. Consider if you have a.h, b.h and z.c, z.c needs b.h
> but not a.h, and now b.h includes a.h ("for convenience") -- changing
> a.h would needlessly recompile z.c. Now, changing slab.h will probably
> recompile everything anyway, but still...

In my configuration after touching slab.h and recompilation:

$ find -name '*.o' -newer ../linux3/include/linux/slab.h |wc -l
1508
$ find -name '*.o' |wc -l
1928

78% object files were recompiled, 22% were no recompiled.  Careful use 
of includes does save time.

-- 
Regards,
Pavel Roskin

Re: [PATCH -next] bcma: main.c needs to include <linux/slab.h>

[Rafał Miłecki]

Hopefully last thing ;)

2011/6/26 Geert Uytterhoeven <geert@linux-m68k.org>:
> m68k allmodconfig:
>
> drivers/bcma/main.c: In function ‘bcma_release_core_dev’:
> drivers/bcma/main.c:68: error: implicit declaration of function ‘kfree’
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> --


If this is not manual mistake, you may want to fix your script or
whatever you use. You typed "--" instead of "---", which resulted in
the following commit message:



m68k allmodconfig:

 drivers/bcma/main.c: In function ‘bcma_release_core_dev’:
 drivers/bcma/main.c:68: error: implicit declaration of function ‘kfree’

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
 --
 http://kisskb.ellerman.id.au/kisskb/buildresult/4243344/

  drivers/bcma/main.c |    1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
Signed-off-by: John W. Linville <linville@tuxdriver.com>



-- 
Rafał

Re: [PATCH -next] bcma: main.c needs to include <linux/slab.h>

[Geert Uytterhoeven]

2011/6/28 Rafał Miłecki <zajec5@gmail.com>:
> Hopefully last thing ;)
>
> 2011/6/26 Geert Uytterhoeven <geert@linux-m68k.org>:
>> m68k allmodconfig:
>>
>> drivers/bcma/main.c: In function ‘bcma_release_core_dev’:
>> drivers/bcma/main.c:68: error: implicit declaration of function ‘kfree’
>>
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> --
>
>
> If this is not manual mistake, you may want to fix your script or
> whatever you use. You typed "--" instead of "---", which resulted in
> the following commit message:

It's a manual mistake :-)

> m68k allmodconfig:
>
>  drivers/bcma/main.c: In function ‘bcma_release_core_dev’:
>  drivers/bcma/main.c:68: error: implicit declaration of function ‘kfree’
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>  --
>  http://kisskb.ellerman.id.au/kisskb/buildresult/4243344/
>
>  drivers/bcma/main.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> Signed-off-by: John W. Linville <linville@tuxdriver.com>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds