Patchwork ARM: localtimer: add header linux/errno.h explicitly

login
register
mail settings
Submitter Shawn Guo
Date Sept. 23, 2011, 5:22 p.m.
Message ID <1316798545-21128-1-git-send-email-shawn.guo@linaro.org>
Download mbox | patch
Permalink /patch/116157/
State New
Headers show

Comments

Shawn Guo - Sept. 23, 2011, 5:22 p.m.
Per the text in  Documentation/SubmitChecklist as below, we should
explicitly have header linux/errno.h in localtimer.h for ENXIO
reference.

1: If you use a facility then #include the file that defines/declares
   that facility.  Don't depend on other header files pulling in ones
   that you use.

Otherwise, we may run into some compiling error like the following one,
if any file includes localtimer.h without CONFIG_LOCAL_TIMERS defined.

  arch/arm/include/asm/localtimer.h: In function ‘local_timer_setup’:
  arch/arm/include/asm/localtimer.h:53:10: error: ‘ENXIO’ undeclared (first use in this function)

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 arch/arm/include/asm/localtimer.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Shawn Guo - Sept. 30, 2011, 2:02 a.m.
Hi Russell,

Do you have any comment on this patch?  Otherwise, I will put it into
patch tracker.

Regards,
Shawn

On Sat, Sep 24, 2011 at 01:22:25AM +0800, Shawn Guo wrote:
> Per the text in  Documentation/SubmitChecklist as below, we should
> explicitly have header linux/errno.h in localtimer.h for ENXIO
> reference.
> 
> 1: If you use a facility then #include the file that defines/declares
>    that facility.  Don't depend on other header files pulling in ones
>    that you use.
> 
> Otherwise, we may run into some compiling error like the following one,
> if any file includes localtimer.h without CONFIG_LOCAL_TIMERS defined.
> 
>   arch/arm/include/asm/localtimer.h: In function ‘local_timer_setup’:
>   arch/arm/include/asm/localtimer.h:53:10: error: ‘ENXIO’ undeclared (first use in this function)
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  arch/arm/include/asm/localtimer.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/include/asm/localtimer.h b/arch/arm/include/asm/localtimer.h
> index 080d74f..698ff73 100644
> --- a/arch/arm/include/asm/localtimer.h
> +++ b/arch/arm/include/asm/localtimer.h
> @@ -48,6 +48,8 @@ int local_timer_setup(struct clock_event_device *);
>  
>  #else
>  
> +#include <linux/errno.h>
> +
>  static inline int local_timer_setup(struct clock_event_device *evt)
>  {
>  	return -ENXIO;
> -- 
> 1.7.4.1
Russell King - ARM Linux - Oct. 1, 2011, 4:37 p.m.
On Fri, Sep 30, 2011 at 10:02:10AM +0800, Shawn Guo wrote:
> Hi Russell,
> 
> Do you have any comment on this patch?  Otherwise, I will put it into
> patch tracker.

Only that the include should be towards the top of the file rather
than conditionally included.  That avoids potential surprise compile
errors caused by changes in configuration.  (Ok, you may argue that
they shouldn't happen but with the amount of includes we have it's
very difficult to ensure that everything explicitly includes everything
it actually needs.)

Patch

diff --git a/arch/arm/include/asm/localtimer.h b/arch/arm/include/asm/localtimer.h
index 080d74f..698ff73 100644
--- a/arch/arm/include/asm/localtimer.h
+++ b/arch/arm/include/asm/localtimer.h
@@ -48,6 +48,8 @@  int local_timer_setup(struct clock_event_device *);
 
 #else
 
+#include <linux/errno.h>
+
 static inline int local_timer_setup(struct clock_event_device *evt)
 {
 	return -ENXIO;