[PATCH 1/4] include/linux/printk.h is not self-contained

[PATCH 1/4] include/linux/printk.h is not self-contained

[y]

From: Geert Uytterhoeven <geert@linux-m68k.org>

<linux/printk.h> needs to include
  - <stdarg.h> for "va_list",
  - <linux/linkage.h> for "asmlinkage",
  - <linux/types.h> for "bool".

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 include/linux/printk.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index ee048e7..6388bc8 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -1,6 +1,10 @@
 #ifndef __KERNEL_PRINTK__
 #define __KERNEL_PRINTK__
 
+#include <stdarg.h>
+#include <linux/linkage.h>
+#include <linux/types.h>
+
 extern const char linux_banner[];
 extern const char linux_proc_banner[];
 
-- 
1.7.0.4

Re: [PATCH 1/4] include/linux/printk.h is not self-contained

[Joe Perches]

On Wed, 2011-05-25 at 11:09 +0200, y@mail.of.borg wrote:
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> <linux/printk.h> needs to include
>   - <stdarg.h> for "va_list",
>   - <linux/linkage.h> for "asmlinkage",
>   - <linux/types.h> for "bool".

I wonder if printk should be self-contained.

If so, this patch should also add:

#include <linux/dynamic_debug.h>
#include <linux/ratelimit.h>

If not, maybe add:

#ifndef _LINUX_KERNEL_H
#error don't include <linux/printk.h> directly, use <linux/kernel.h>
#endif

Also asmlinkage is an ancient artifact and should
be removed from the declarations of early_printk,
vprintk, and printk.

Re: [PATCH 1/4] include/linux/printk.h is not self-contained

[Geert Uytterhoeven]

On Wed, May 25, 2011 at 16:42, Joe Perches <joe@perches.com> wrote:
> On Wed, 2011-05-25 at 11:09 +0200, y@mail.of.borg wrote:

Sorry, that email address is a relic from typing one too many "y" responses
to git-send-email questions.

>> From: Geert Uytterhoeven <geert@linux-m68k.org>
>> <linux/printk.h> needs to include
>>   - <stdarg.h> for "va_list",
>>   - <linux/linkage.h> for "asmlinkage",
>>   - <linux/types.h> for "bool".
>
> I wonder if printk should be self-contained.
>
> If so, this patch should also add:
>
> #include <linux/dynamic_debug.h>
> #include <linux/ratelimit.h>

Hmm, it does compile without those, as long as you don't use
dynamic debug or ratelimit.

> If not, maybe add:
>
> #ifndef _LINUX_KERNEL_H
> #error don't include <linux/printk.h> directly, use <linux/kernel.h>
> #endif

That's another possibility. We already have several users, though.

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 1/4] include/linux/printk.h is not self-contained

[Alexey Dobriyan]

On Wed, May 25, 2011 at 6:20 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>> If not, maybe add:
>>
>> #ifndef _LINUX_KERNEL_H
>> #error don't include <linux/printk.h> directly, use <linux/kernel.h>
>> #endif
>
> That's another possibility. We already have several users, though.

Eh, it is because of sheer amount of printk() users headers aren't switched,
from kernel.h to printk.h

[PATCH 0/4] printk: cleanups of direct includes

[Joe Perches]

printk.h is a subfile of kernel.h, do not allow it to
be included directly.

Joe Perches (4):
  treewide: Remove direct includes of printk.h
  staging: Remove direct includes of printk.h
  printk: Don't allow direct inclusion
  treewide: Remove asmlinkage from printk

 arch/arm/kernel/early_printk.c                |    2 +-
 arch/ia64/kvm/vmm.c                           |    2 +-
 arch/x86/kernel/early_printk.c                |    2 +-
 drivers/staging/brcm80211/brcmfmac/dhd_sdio.c |    1 -
 drivers/staging/brcm80211/util/bcmutils.c     |    1 -
 drivers/staging/usbip/usbip_common.h          |    2 +-
 fs/ext4/inode.c                               |    1 -
 include/linux/oprofile.h                      |    2 +-
 include/linux/printk.h                        |   10 +++++++---
 kernel/printk.c                               |    6 +++---
 kernel/sysctl.c                               |    3 +--
 11 files changed, 16 insertions(+), 16 deletions(-)

-- 
1.7.4.rc3

[PATCH 1/4] treewide: Remove direct includes of printk.h

[Joe Perches]

Make the uses kernel.h instead.

Signed-off-by: Joe Perches <joe@perches.com>
---
 fs/ext4/inode.c          |    1 -
 include/linux/oprofile.h |    2 +-
 kernel/sysctl.c          |    3 +--
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 2e95819..cffecac 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -39,7 +39,6 @@
 #include <linux/bio.h>
 #include <linux/workqueue.h>
 #include <linux/kernel.h>
-#include <linux/printk.h>
 #include <linux/slab.h>
 #include <linux/ratelimit.h>
 
diff --git a/include/linux/oprofile.h b/include/linux/oprofile.h
index 7f5cfd3..ef7d5b1 100644
--- a/include/linux/oprofile.h
+++ b/include/linux/oprofile.h
@@ -14,10 +14,10 @@
 #define OPROFILE_H
 
 #include <linux/types.h>
+#include <linux/kernel.h>
 #include <linux/spinlock.h>
 #include <linux/init.h>
 #include <linux/errno.h>
-#include <linux/printk.h>
 #include <asm/atomic.h>
  
 /* Each escaped entry is prefixed by ESCAPE_CODE
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 4fc9244..814d227 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -24,14 +24,13 @@
 #include <linux/slab.h>
 #include <linux/sysctl.h>
 #include <linux/signal.h>
-#include <linux/printk.h>
+#include <linux/kernel.h>
 #include <linux/proc_fs.h>
 #include <linux/security.h>
 #include <linux/ctype.h>
 #include <linux/kmemcheck.h>
 #include <linux/fs.h>
 #include <linux/init.h>
-#include <linux/kernel.h>
 #include <linux/kobject.h>
 #include <linux/net.h>
 #include <linux/sysrq.h>
-- 
1.7.4.rc3

[PATCH 2/4] staging: Remove direct includes of printk.h

[Joe Perches]

Make the uses kernel.h instead.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/staging/brcm80211/brcmfmac/dhd_sdio.c |    1 -
 drivers/staging/brcm80211/util/bcmutils.c     |    1 -
 drivers/staging/usbip/usbip_common.h          |    2 +-
 3 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/brcm80211/brcmfmac/dhd_sdio.c b/drivers/staging/brcm80211/brcmfmac/dhd_sdio.c
index a71c6f8..b00029a 100644
--- a/drivers/staging/brcm80211/brcmfmac/dhd_sdio.c
+++ b/drivers/staging/brcm80211/brcmfmac/dhd_sdio.c
@@ -16,7 +16,6 @@
 
 #include <linux/types.h>
 #include <linux/kernel.h>
-#include <linux/printk.h>
 #include <linux/pci_ids.h>
 #include <linux/netdevice.h>
 #include <bcmdefs.h>
diff --git a/drivers/staging/brcm80211/util/bcmutils.c b/drivers/staging/brcm80211/util/bcmutils.c
index 43e5bb3..1c8b6c6 100644
--- a/drivers/staging/brcm80211/util/bcmutils.c
+++ b/drivers/staging/brcm80211/util/bcmutils.c
@@ -21,7 +21,6 @@
 #include <linux/pci.h>
 #include <linux/netdevice.h>
 #include <linux/sched.h>
-#include <linux/printk.h>
 #include <bcmdefs.h>
 #include <stdarg.h>
 #include <bcmutils.h>
diff --git a/drivers/staging/usbip/usbip_common.h b/drivers/staging/usbip/usbip_common.h
index 4a641c5..0f9176d 100644
--- a/drivers/staging/usbip/usbip_common.h
+++ b/drivers/staging/usbip/usbip_common.h
@@ -24,7 +24,7 @@
 #include <linux/device.h>
 #include <linux/interrupt.h>
 #include <linux/net.h>
-#include <linux/printk.h>
+#include <linux/kernel.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
 #include <linux/usb.h>
-- 
1.7.4.rc3

Re: [PATCH 2/4] staging: Remove direct includes of printk.h

[Roland Vossen]

On 05/25/2011 07:02 PM, Joe Perches wrote:
> Make the uses kernel.h instead.
>
> Signed-off-by: Joe Perches<joe@perches.com>

Acked-by: Roland Vossen <rvossen@broadcom.com>

Re: [PATCH 2/4] staging: Remove direct includes of printk.h

[julie Sullivan]

> --- a/drivers/staging/brcm80211/brcmfmac/dhd_sdio.c
> +++ b/drivers/staging/brcm80211/brcmfmac/dhd_sdio.c
> @@ -16,7 +16,6 @@
>
>  #include <linux/types.h>
>  #include <linux/kernel.h>
> -#include <linux/printk.h>


> --- a/drivers/staging/usbip/usbip_common.h
> +++ b/drivers/staging/usbip/usbip_common.h
> @@ -24,7 +24,7 @@
>  #include <linux/device.h>
>  #include <linux/interrupt.h>
>  #include <linux/net.h>
> -#include <linux/printk.h>
> +#include <linux/kernel.h>
>  #include <linux/spinlock.h>
>  #include <linux/types.h>

types.h is also a subfile of kernel.h, right?
Or maybe you patched this one already - if so please excuse the noise :-)

Cheers
Julie

Re: [PATCH 2/4] staging: Remove direct includes of printk.h

[Joe Perches]

On Sun, 2011-05-29 at 18:00 +0100, julie Sullivan wrote: 
> > --- a/drivers/staging/brcm80211/brcmfmac/dhd_sdio.c
[] 
> types.h is also a subfile of kernel.h, right?

Yes it is.

> Or maybe you patched this one already - if so please excuse the noise :-)

No I didn't patch that one.

I think it wouldn't be good to do that.
printk.h may be a special case because it was
moved out of kernel.h just to make it a bit neater.

You are welcome to if you choose to.

There are like 1500 or so files that have both kernel.h and types.h

$ grep --include=*.[ch] -rP -l "^\s*\#\s*include\s+\<linux/types\.h>" * | \
  xargs grep -Pl "^\s*\#\s*include\s+\<linux/kernel\.h\>" | wc -l
1572

[PATCH 3/4] printk: Don't allow direct inclusion

[Joe Perches]

printk.h is a subfile of kernel.h, do not allow
it to be included directly.

Signed-off-by: Joe Perches <joe@perches.com>
---
 include/linux/printk.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index ee048e7..3736545 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -1,3 +1,7 @@
+#ifndef _LINUX_KERNEL_H
+#error "Do not include <linux/printk.h>, use <linux/kernel.h> instead"
+#endif
+
 #ifndef __KERNEL_PRINTK__
 #define __KERNEL_PRINTK__
 
-- 
1.7.4.rc3

[PATCH 4/4] treewide: Remove asmlinkage from printk

[Joe Perches]

Remove the now unnecessary asmlinkage attribute from the various
printk prototypes and uses.

Signed-off-by: Joe Perches <joe@perches.com>
---
 arch/arm/kernel/early_printk.c |    2 +-
 arch/ia64/kvm/vmm.c            |    2 +-
 arch/x86/kernel/early_printk.c |    2 +-
 include/linux/printk.h         |    6 +++---
 kernel/printk.c                |    6 +++---
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm/kernel/early_printk.c b/arch/arm/kernel/early_printk.c
index 85aa2b2..46798d5 100644
--- a/arch/arm/kernel/early_printk.c
+++ b/arch/arm/kernel/early_printk.c
@@ -36,7 +36,7 @@ static struct console early_console = {
 	.index =	-1,
 };
 
-asmlinkage void early_printk(const char *fmt, ...)
+void early_printk(const char *fmt, ...)
 {
 	char buf[512];
 	int n;
diff --git a/arch/ia64/kvm/vmm.c b/arch/ia64/kvm/vmm.c
index f0b9cac..40331ac 100644
--- a/arch/ia64/kvm/vmm.c
+++ b/arch/ia64/kvm/vmm.c
@@ -81,7 +81,7 @@ static void vcpu_debug_exit(struct kvm_vcpu *vcpu)
 	local_irq_restore(psr);
 }
 
-asmlinkage int printk(const char *fmt, ...)
+int printk(const char *fmt, ...)
 {
 	struct kvm_vcpu *vcpu = current_vcpu;
 	va_list args;
diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
index cd28a35..b9b323a 100644
--- a/arch/x86/kernel/early_printk.c
+++ b/arch/x86/kernel/early_printk.c
@@ -173,7 +173,7 @@ static struct console early_serial_console = {
 static struct console *early_console = &early_vga_console;
 static int __initdata early_console_initialized;
 
-asmlinkage void early_printk(const char *fmt, ...)
+void early_printk(const char *fmt, ...)
 {
 	char buf[512];
 	int n;
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 3736545..e52bc4b 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -90,16 +90,16 @@ int no_printk(const char *fmt, ...)
 	return 0;
 }
 
-extern asmlinkage __attribute__ ((format (printf, 1, 2)))
+extern __attribute__ ((format (printf, 1, 2)))
 void early_printk(const char *fmt, ...);
 
 extern int printk_needs_cpu(int cpu);
 extern void printk_tick(void);
 
 #ifdef CONFIG_PRINTK
-asmlinkage __attribute__ ((format (printf, 1, 0)))
+__attribute__ ((format (printf, 1, 0)))
 int vprintk(const char *fmt, va_list args);
-asmlinkage __attribute__ ((format (printf, 1, 2))) __cold
+__attribute__ ((format (printf, 1, 2))) __cold
 int printk(const char *fmt, ...);
 
 /*
diff --git a/kernel/printk.c b/kernel/printk.c
index da8ca81..735056c 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -46,7 +46,7 @@
 /*
  * Architectures can override it:
  */
-void asmlinkage __attribute__((weak)) early_printk(const char *fmt, ...)
+void __attribute__((weak)) early_printk(const char *fmt, ...)
 {
 }
 
@@ -704,7 +704,7 @@ static int have_callable_console(void)
  * See the vsnprintf() documentation for format string extensions over C99.
  */
 
-asmlinkage int printk(const char *fmt, ...)
+int printk(const char *fmt, ...)
 {
 	va_list args;
 	int r;
@@ -794,7 +794,7 @@ static inline void printk_delay(void)
 	}
 }
 
-asmlinkage int vprintk(const char *fmt, va_list args)
+int vprintk(const char *fmt, va_list args)
 {
 	int printed_len = 0;
 	int current_log_level = default_message_loglevel;
-- 
1.7.4.rc3

Re: [PATCH 1/4] include/linux/printk.h is not self-contained

[Randy Dunlap]

On 05/25/11 02:09, y@mail.of.borg wrote:
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> 
> <linux/printk.h> needs to include
>   - <stdarg.h> for "va_list",
>   - <linux/linkage.h> for "asmlinkage",
>   - <linux/types.h> for "bool".
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>  include/linux/printk.h |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index ee048e7..6388bc8 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -1,6 +1,10 @@
>  #ifndef __KERNEL_PRINTK__
>  #define __KERNEL_PRINTK__
>  
> +#include <stdarg.h>
> +#include <linux/linkage.h>
> +#include <linux/types.h>
> +
>  extern const char linux_banner[];
>  extern const char linux_proc_banner[];
>  

I agree with this patch along with Joe's 2 header additions...
but I get build errors with it (as reported yesterday):


  CC      arch/x86/kernel/asm-offsets.s
In file included from linux-next-20110524/include/linux/thread_info.h:53,
                 from linux-next-20110524/include/linux/preempt.h:9,
                 from linux-next-20110524/include/linux/spinlock.h:50,
                 from linux-next-20110524/include/linux/ratelimit.h:5,
                 from linux-next-20110524/include/linux/printk.h:8,
                 from linux-next-20110524/include/linux/kernel.h:20,
                 from linux-next-20110524/arch/x86/include/asm/percpu.h:44,
                 from linux-next-20110524/arch/x86/include/asm/current.h:5,
                 from linux-next-20110524/arch/x86/include/asm/processor.h:15,
                 from linux-next-20110524/arch/x86/include/asm/atomic.h:6,
                 from linux-next-20110524/include/linux/crypto.h:20,
                 from linux-next-20110524/arch/x86/kernel/asm-offsets.c:8:
linux-next-20110524/arch/x86/include/asm/thread_info.h:34: error: expected specifier-qualifier-list before 'mm_segment_t'
In file included from linux-next-20110524/include/linux/ratelimit.h:5,
                 from linux-next-20110524/include/linux/printk.h:8,
                 from linux-next-20110524/include/linux/kernel.h:20,
                 from linux-next-20110524/arch/x86/include/asm/percpu.h:44,
                 from linux-next-20110524/arch/x86/include/asm/current.h:5,
                 from linux-next-20110524/arch/x86/include/asm/processor.h:15,
                 from linux-next-20110524/arch/x86/include/asm/atomic.h:6,
                 from linux-next-20110524/include/linux/crypto.h:20,
                 from linux-next-20110524/arch/x86/kernel/asm-offsets.c:8:
linux-next-20110524/include/linux/spinlock.h: In function 'spin_unlock_wait':
linux-next-20110524/include/linux/spinlock.h:360: error: implicit declaration of function 'cpu_relax'
In file included from linux-next-20110524/arch/x86/include/asm/atomic.h:6,
                 from linux-next-20110524/include/linux/crypto.h:20,
                 from linux-next-20110524/arch/x86/kernel/asm-offsets.c:8:
linux-next-20110524/arch/x86/include/asm/processor.h: At top level:
linux-next-20110524/arch/x86/include/asm/processor.h:707: warning: conflicting types for 'cpu_relax'
linux-next-20110524/arch/x86/include/asm/processor.h:707: error: static declaration of 'cpu_relax' follows non-static declaration
linux-next-20110524/include/linux/spinlock.h:360: note: previous implicit declaration of 'cpu_relax' was here
In file included from linux-next-20110524/arch/x86/include/asm/i387.h:17,
                 from linux-next-20110524/arch/x86/include/asm/suspend_32.h:10,
                 from linux-next-20110524/arch/x86/include/asm/suspend.h:2,
                 from linux-next-20110524/arch/x86/kernel/asm-offsets.c:18:
linux-next-20110524/include/linux/regset.h: In function 'copy_regset_to_user':
linux-next-20110524/include/linux/regset.h:338: error: 'struct thread_info' has no member named 'addr_limit'
linux-next-20110524/include/linux/regset.h:338: error: 'struct thread_info' has no member named 'addr_limit'
linux-next-20110524/include/linux/regset.h:338: error: 'struct thread_info' has no member named 'addr_limit'
linux-next-20110524/include/linux/regset.h:338: error: 'struct thread_info' has no member named 'addr_limit'
linux-next-20110524/include/linux/regset.h:338: error: 'struct thread_info' has no member named 'addr_limit'
linux-next-20110524/include/linux/regset.h:338: error: 'struct thread_info' has no member named 'addr_limit'
linux-next-20110524/include/linux/regset.h:338: error: 'struct thread_info' has no member named 'addr_limit'
linux-next-20110524/include/linux/regset.h:338: error: 'struct thread_info' has no member named 'addr_limit'
linux-next-20110524/include/linux/regset.h:338: error: 'struct thread_info' has no member named 'addr_limit'
linux-next-20110524/include/linux/regset.h: In function 'copy_regset_from_user':
linux-next-20110524/include/linux/regset.h:361: error: 'struct thread_info' has no member named 'addr_limit'
linux-next-20110524/include/linux/regset.h:361: error: 'struct thread_info' has no member named 'addr_limit'
linux-next-20110524/include/linux/regset.h:361: error: 'struct thread_info' has no member named 'addr_limit'
linux-next-20110524/include/linux/regset.h:361: error: 'struct thread_info' has no member named 'addr_limit'
linux-next-20110524/include/linux/regset.h:361: error: 'struct thread_info' has no member named 'addr_limit'
linux-next-20110524/include/linux/regset.h:361: error: 'struct thread_info' has no member named 'addr_limit'
linux-next-20110524/include/linux/regset.h:361: error: 'struct thread_info' has no member named 'addr_limit'
linux-next-20110524/include/linux/regset.h:361: error: 'struct thread_info' has no member named 'addr_limit'
linux-next-20110524/include/linux/regset.h:361: error: 'struct thread_info' has no member named 'addr_limit'
In file included from linux-next-20110524/arch/x86/kernel/asm-offsets.c:25:
linux-next-20110524/arch/x86/kernel/asm-offsets_32.c: In function 'foo':
linux-next-20110524/arch/x86/kernel/asm-offsets_32.c:32: error: 'struct thread_info' has no member named 'sysenter_return'
linux-next-20110524/arch/x86/kernel/asm-offsets.c: In function 'common':
linux-next-20110524/arch/x86/kernel/asm-offsets.c:34: error: 'struct thread_info' has no member named 'addr_limit'

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***