[PATCH] thinkpad_acpi: Remove side effects from vdbg_printk -> no_printk macro

[PATCH] thinkpad_acpi: Remove side effects from vdbg_printk -> no_printk macro

[Joe Perches]

vdbg_printk when not using CONFIG_THINKPAD_ACPI_DEBUG uses
no_printk which produces no logging output but always
evaluates arguments.

Change the macro to surround the no_printk call with
	do { if (0) no_printk(...); } while (0)
to avoid the unnecessary argument evaluations.

$ size drivers/platform/x86/thinkpad_acpi.o*
   text	   data	    bss	    dec	    hex	filename
  60918	   6184	    824	  67926	  10956	drivers/platform/x86/thinkpad_acpi.o.new
  60927	   6184	    824	  67935	  1095f	drivers/platform/x86/thinkpad_acpi.o.old

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 33e488c..131dd74 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -402,7 +402,7 @@ static const char *str_supported(int is_supported);
 #else
 static inline const char *str_supported(int is_supported) { return ""; }
 #define vdbg_printk(a_dbg_level, format, arg...)	\
-	no_printk(format, ##arg)
+	do { if (0) no_printk(format, ##arg); } while (0)
 #endif
 
 static void tpacpi_log_usertask(const char * const what)

Re: [PATCH] thinkpad_acpi: Remove side effects from vdbg_printk -> no_printk macro

[Henrique de Moraes Holschuh]

On Wed, Aug 26, 2015, at 15:13, Joe Perches wrote:
> vdbg_printk when not using CONFIG_THINKPAD_ACPI_DEBUG uses
> no_printk which produces no logging output but always
> evaluates arguments.
> 
> Change the macro to surround the no_printk call with
> 	do { if (0) no_printk(...); } while (0)
> to avoid the unnecessary argument evaluations.
> 
> $ size drivers/platform/x86/thinkpad_acpi.o*
>    text       data     bss     dec     hex filename
>   60918      6184     824   67926   10956
>   drivers/platform/x86/thinkpad_acpi.o.new
>   60927      6184     824   67935   1095f
>   drivers/platform/x86/thinkpad_acpi.o.old

The code size savings of this change are really small...

> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -402,7 +402,7 @@ static const char *str_supported(int is_supported);
>  #else
>  static inline const char *str_supported(int is_supported) { return ""; }
>  #define vdbg_printk(a_dbg_level, format, arg...)        \
> -       no_printk(format, ##arg)
> +       do { if (0) no_printk(format, ##arg); } while (0)
>  #endif

And won't this change disable compile-time checking of 'format ## arg'
for issues?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH] thinkpad_acpi: Remove side effects from vdbg_printk -> no_printk macro

[Joe Perches]

On Wed, 2015-08-26 at 15:27 -0300, Henrique de Moraes Holschuh wrote:
> On Wed, Aug 26, 2015, at 15:13, Joe Perches wrote:
> > vdbg_printk when not using CONFIG_THINKPAD_ACPI_DEBUG uses
> > no_printk which produces no logging output but always
> > evaluates arguments.
> > 
> > Change the macro to surround the no_printk call with
> > 	do { if (0) no_printk(...); } while (0)
> > to avoid the unnecessary argument evaluations.
> > 
> > $ size drivers/platform/x86/thinkpad_acpi.o*
> >    text       data     bss     dec     hex filename
> >   60918      6184     824   67926   10956
> >   drivers/platform/x86/thinkpad_acpi.o.new
> >   60927      6184     824   67935   1095f
> >   drivers/platform/x86/thinkpad_acpi.o.old
> 
> The code size savings of this change are really small...

yup.  This is just an on-the-way patch to get
all the no_printk uses in a state that can
be converted more simply to a side-effect free
mechanism.

> > --- a/drivers/platform/x86/thinkpad_acpi.c
> > +++ b/drivers/platform/x86/thinkpad_acpi.c
> > @@ -402,7 +402,7 @@ static const char *str_supported(int is_supported);
> >  #else
> >  static inline const char *str_supported(int is_supported) { return ""; }
> >  #define vdbg_printk(a_dbg_level, format, arg...)        \
> > -       no_printk(format, ##arg)
> > +       do { if (0) no_printk(format, ##arg); } while (0)
> >  #endif
> 
> And won't this change disable compile-time checking of 'format ## arg'
> for issues?

No.  The call is still there so the compiler checks
the format/argument matches, but eliminates the
call and any side-effect calls from the object file.

Re: [PATCH] thinkpad_acpi: Remove side effects from vdbg_printk -> no_printk macro

[Henrique de Moraes Holschuh]

On Wed, 26 Aug 2015, Joe Perches wrote:
> On Wed, 2015-08-26 at 15:27 -0300, Henrique de Moraes Holschuh wrote:
> > On Wed, Aug 26, 2015, at 15:13, Joe Perches wrote:
> > > vdbg_printk when not using CONFIG_THINKPAD_ACPI_DEBUG uses
> > > no_printk which produces no logging output but always
> > > evaluates arguments.
> > > 
> > > Change the macro to surround the no_printk call with
> > > 	do { if (0) no_printk(...); } while (0)
> > > to avoid the unnecessary argument evaluations.
> > > 
> > > $ size drivers/platform/x86/thinkpad_acpi.o*
> > >    text       data     bss     dec     hex filename
> > >   60918      6184     824   67926   10956
> > >   drivers/platform/x86/thinkpad_acpi.o.new
> > >   60927      6184     824   67935   1095f
> > >   drivers/platform/x86/thinkpad_acpi.o.old
> > 
> > The code size savings of this change are really small...
> 
> yup.  This is just an on-the-way patch to get
> all the no_printk uses in a state that can
> be converted more simply to a side-effect free
> mechanism.
> 
> > > --- a/drivers/platform/x86/thinkpad_acpi.c
> > > +++ b/drivers/platform/x86/thinkpad_acpi.c
> > > @@ -402,7 +402,7 @@ static const char *str_supported(int is_supported);
> > >  #else
> > >  static inline const char *str_supported(int is_supported) { return ""; }
> > >  #define vdbg_printk(a_dbg_level, format, arg...)        \
> > > -       no_printk(format, ##arg)
> > > +       do { if (0) no_printk(format, ##arg); } while (0)
> > >  #endif
> > 
> > And won't this change disable compile-time checking of 'format ## arg'
> > for issues?
> 
> No.  The call is still there so the compiler checks
> the format/argument matches, but eliminates the
> call and any side-effect calls from the object file.

Ok, very well.  In that case, I will ack it.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH] thinkpad_acpi: Remove side effects from vdbg_printk -> no_printk macro

[Henrique de Moraes Holschuh]

On Wed, 26 Aug 2015, Joe Perches wrote:
> vdbg_printk when not using CONFIG_THINKPAD_ACPI_DEBUG uses
> no_printk which produces no logging output but always
> evaluates arguments.
> 
> Change the macro to surround the no_printk call with
> 	do { if (0) no_printk(...); } while (0)
> to avoid the unnecessary argument evaluations.
> 
> $ size drivers/platform/x86/thinkpad_acpi.o*
>    text	   data	    bss	    dec	    hex	filename
>   60918	   6184	    824	  67926	  10956	drivers/platform/x86/thinkpad_acpi.o.new
>   60927	   6184	    824	  67935	  1095f	drivers/platform/x86/thinkpad_acpi.o.old
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

> ---
>  drivers/platform/x86/thinkpad_acpi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 33e488c..131dd74 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -402,7 +402,7 @@ static const char *str_supported(int is_supported);
>  #else
>  static inline const char *str_supported(int is_supported) { return ""; }
>  #define vdbg_printk(a_dbg_level, format, arg...)	\
> -	no_printk(format, ##arg)
> +	do { if (0) no_printk(format, ##arg); } while (0)
>  #endif
>  
>  static void tpacpi_log_usertask(const char * const what)
> 

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH] thinkpad_acpi: Remove side effects from vdbg_printk -> no_printk macro

[Darren Hart]

On Thu, Aug 27, 2015 at 02:33:06PM -0300, Henrique de Moraes Holschuh wrote:
> On Wed, 26 Aug 2015, Joe Perches wrote:
> > vdbg_printk when not using CONFIG_THINKPAD_ACPI_DEBUG uses
> > no_printk which produces no logging output but always
> > evaluates arguments.
> > 
> > Change the macro to surround the no_printk call with
> > 	do { if (0) no_printk(...); } while (0)
> > to avoid the unnecessary argument evaluations.
> > 
> > $ size drivers/platform/x86/thinkpad_acpi.o*
> >    text	   data	    bss	    dec	    hex	filename
> >   60918	   6184	    824	  67926	  10956	drivers/platform/x86/thinkpad_acpi.o.new
> >   60927	   6184	    824	  67935	  1095f	drivers/platform/x86/thinkpad_acpi.o.old
> > 
> > Signed-off-by: Joe Perches <joe@perches.com>
> 
> Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

Queued, thanks.

-- 
Darren Hart
Intel Open Source Technology Center