| Message ID | alpine.DEB.2.00.1106261016100.25900@ayla.of.borg (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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?
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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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.
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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
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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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>
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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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");
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(-)