LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Fix empty macros in acpi.
@ 2007-06-12 23:33 Dave Jones
  2007-06-13  0:00 ` Al Viro
  0 siblings, 1 reply; 7+ messages in thread
From: Dave Jones @ 2007-06-12 23:33 UTC (permalink / raw)
  To: Linux Kernel

ACPI has a ton of macros which make a bunch of empty if's
when configured in non-debug mode.

Signed-off-by: Dave Jones <davej@redhat.com>

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 4334c20..5955cfd 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -16,7 +16,7 @@
 #if ACPI_GLUE_DEBUG
 #define DBG(x...) printk(PREFIX x)
 #else
-#define DBG(x...)
+#define DBG(x...) do { } while(0)
 #endif
 static LIST_HEAD(bus_type_list);
 static DECLARE_RWSEM(bus_type_sem);
diff --git a/include/acpi/acmacros.h b/include/acpi/acmacros.h
index 8948a64..e64a748 100644
--- a/include/acpi/acmacros.h
+++ b/include/acpi/acmacros.h
@@ -599,26 +599,26 @@
 #define ACPI_DEBUG_EXEC(a)
 #define ACPI_NORMAL_EXEC(a)             a;
 
-#define ACPI_DEBUG_DEFINE(a)
-#define ACPI_DEBUG_ONLY_MEMBERS(a)
-#define ACPI_FUNCTION_NAME(a)
-#define ACPI_FUNCTION_TRACE(a)
-#define ACPI_FUNCTION_TRACE_PTR(a,b)
-#define ACPI_FUNCTION_TRACE_U32(a,b)
-#define ACPI_FUNCTION_TRACE_STR(a,b)
-#define ACPI_FUNCTION_EXIT
-#define ACPI_FUNCTION_STATUS_EXIT(s)
-#define ACPI_FUNCTION_VALUE_EXIT(s)
-#define ACPI_FUNCTION_ENTRY()
-#define ACPI_DUMP_STACK_ENTRY(a)
-#define ACPI_DUMP_OPERANDS(a,b,c,d,e)
-#define ACPI_DUMP_ENTRY(a,b)
-#define ACPI_DUMP_TABLES(a,b)
-#define ACPI_DUMP_PATHNAME(a,b,c,d)
-#define ACPI_DUMP_RESOURCE_LIST(a)
-#define ACPI_DUMP_BUFFER(a,b)
-#define ACPI_DEBUG_PRINT(pl)
-#define ACPI_DEBUG_PRINT_RAW(pl)
+#define ACPI_DEBUG_DEFINE(a)		do { } while(0)
+#define ACPI_DEBUG_ONLY_MEMBERS(a)	do { } while(0)
+#define ACPI_FUNCTION_NAME(a)		do { } while(0)
+#define ACPI_FUNCTION_TRACE(a)		do { } while(0)
+#define ACPI_FUNCTION_TRACE_PTR(a,b)	do { } while(0)
+#define ACPI_FUNCTION_TRACE_U32(a,b)	do { } while(0)
+#define ACPI_FUNCTION_TRACE_STR(a,b)	do { } while(0)
+#define ACPI_FUNCTION_EXIT		do { } while(0)
+#define ACPI_FUNCTION_STATUS_EXIT(s)	do { } while(0)
+#define ACPI_FUNCTION_VALUE_EXIT(s)	do { } while(0)
+#define ACPI_FUNCTION_ENTRY()		do { } while(0)
+#define ACPI_DUMP_STACK_ENTRY(a)	do { } while(0)
+#define ACPI_DUMP_OPERANDS(a,b,c,d,e)	do { } while(0)
+#define ACPI_DUMP_ENTRY(a,b)		do { } while(0)
+#define ACPI_DUMP_TABLES(a,b)		do { } while(0)
+#define ACPI_DUMP_PATHNAME(a,b,c,d)	do { } while(0)
+#define ACPI_DUMP_RESOURCE_LIST(a)	do { } while(0)
+#define ACPI_DUMP_BUFFER(a,b)		do { } while(0)
+#define ACPI_DEBUG_PRINT(pl)		do { } while(0)
+#define ACPI_DEBUG_PRINT_RAW(pl)	do { } while(0)
 
 #define return_VOID                     return
 #define return_ACPI_STATUS(s)           return(s)



-- 
http://www.codemonkey.org.uk

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Fix empty macros in acpi.
  2007-06-12 23:33 Fix empty macros in acpi Dave Jones
@ 2007-06-13  0:00 ` Al Viro
  2007-06-13  0:21   ` Dave Jones
  0 siblings, 1 reply; 7+ messages in thread
From: Al Viro @ 2007-06-13  0:00 UTC (permalink / raw)
  To: Dave Jones, Linux Kernel

On Tue, Jun 12, 2007 at 07:33:09PM -0400, Dave Jones wrote:
> +#define DBG(x...) do { } while(0)

Eh...  Please, stop it - if you want a function-call-like no-op returning void,
use ((void)0).  At least that way one can say DBG(....),foo(), etc.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Fix empty macros in acpi.
  2007-06-13  0:00 ` Al Viro
@ 2007-06-13  0:21   ` Dave Jones
  2007-06-13  0:26     ` Al Viro
  0 siblings, 1 reply; 7+ messages in thread
From: Dave Jones @ 2007-06-13  0:21 UTC (permalink / raw)
  To: Al Viro; +Cc: Linux Kernel

On Wed, Jun 13, 2007 at 01:00:29AM +0100, Al Viro wrote:
 > On Tue, Jun 12, 2007 at 07:33:09PM -0400, Dave Jones wrote:
 > > +#define DBG(x...) do { } while(0)
 > 
 > Eh...  Please, stop it - if you want a function-call-like no-op returning void,
 > use ((void)0).  At least that way one can say DBG(....),foo(), etc.

They both end up compiled to nothing anyway, so I'm not bothered
either way..   I'm not sure I follow why the syntax of that last part
is a good thing.  It looks like something we'd want to avoid rather
than promote?

	Dave

-- 
http://www.codemonkey.org.uk

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Fix empty macros in acpi.
  2007-06-13  0:21   ` Dave Jones
@ 2007-06-13  0:26     ` Al Viro
  2007-07-03  5:22       ` Len Brown
  0 siblings, 1 reply; 7+ messages in thread
From: Al Viro @ 2007-06-13  0:26 UTC (permalink / raw)
  To: Dave Jones, Linux Kernel

On Tue, Jun 12, 2007 at 08:21:15PM -0400, Dave Jones wrote:
> On Wed, Jun 13, 2007 at 01:00:29AM +0100, Al Viro wrote:
>  > On Tue, Jun 12, 2007 at 07:33:09PM -0400, Dave Jones wrote:
>  > > +#define DBG(x...) do { } while(0)
>  > 
>  > Eh...  Please, stop it - if you want a function-call-like no-op returning void,
>  > use ((void)0).  At least that way one can say DBG(....),foo(), etc.
> 
> They both end up compiled to nothing anyway, so I'm not bothered
> either way..   I'm not sure I follow why the syntax of that last part
> is a good thing.  It looks like something we'd want to avoid rather
> than promote?

If on one side of ifdef it's a void-valued expression, so it should be
on another; the reason is that we don't get surprise differences between
the builds...

IOW, if it doesn't build in some context, it should consistently fail to
build in that context.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Fix empty macros in acpi.
  2007-06-13  0:26     ` Al Viro
@ 2007-07-03  5:22       ` Len Brown
  2007-07-03  5:44         ` Dave Jones
  0 siblings, 1 reply; 7+ messages in thread
From: Len Brown @ 2007-07-03  5:22 UTC (permalink / raw)
  To: Al Viro, Dave Jones; +Cc: Linux Kernel

On Tuesday 12 June 2007 20:26, Al Viro wrote:
> On Tue, Jun 12, 2007 at 08:21:15PM -0400, Dave Jones wrote:
> > On Wed, Jun 13, 2007 at 01:00:29AM +0100, Al Viro wrote:
> >  > On Tue, Jun 12, 2007 at 07:33:09PM -0400, Dave Jones wrote:
> >  > > +#define DBG(x...) do { } while(0)
> >  > 
> >  > Eh...  Please, stop it - if you want a function-call-like no-op returning void,
> >  > use ((void)0).  At least that way one can say DBG(....),foo(), etc.
> > 
> > They both end up compiled to nothing anyway, so I'm not bothered
> > either way..   I'm not sure I follow why the syntax of that last part
> > is a good thing.  It looks like something we'd want to avoid rather
> > than promote?
> 
> If on one side of ifdef it's a void-valued expression, so it should be
> on another; the reason is that we don't get surprise differences between
> the builds...

true, DBG() in this case would expand to printk(), which returns int.

But in practice, DBG isn't used in any expressions -- and the other zillion
places in the kernel where it is used, it is used as in dave's patch.

Indeed, I don't see printk used in expressions either...

While I know it is common Linux style, and by default it is okay with gcc,
it seems somewhat half-baked to call functions and not check their return
value by default.  IMHO, if they return something, it should be checked,
or explicitly ignored -- or it shouldn't return anything in the first place...

> IOW, if it doesn't build in some context, it should consistently fail to
> build in that context.

whelp, it seems that the reason for this patch is this:

#define DBG()

if(...)
	DBG();
next_c_statement

which turns into
if(...) ;
next_c_statement

But since there is an intervening ';', this code is still functionally correct
and a decent compiler will delete the test altogether, yes?

So is there some real problem here that I missed,
or is this to make some code-checking tool that I don't have happy?

thanks,
-Len

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Fix empty macros in acpi.
  2007-07-03  5:22       ` Len Brown
@ 2007-07-03  5:44         ` Dave Jones
  2007-07-22  4:55           ` Len Brown
  0 siblings, 1 reply; 7+ messages in thread
From: Dave Jones @ 2007-07-03  5:44 UTC (permalink / raw)
  To: Len Brown; +Cc: Al Viro, Linux Kernel

On Tue, Jul 03, 2007 at 01:22:47AM -0400, Len Brown wrote:

 > whelp, it seems that the reason for this patch is this:
 > 
 > #define DBG()
 > 
 > if(...)
 > 	DBG();
 > next_c_statement
 > 
 > which turns into
 > if(...) ;
 > next_c_statement
 > 
 > But since there is an intervening ';', this code is still functionally correct
 > and a decent compiler will delete the test altogether, yes?

Right, gcc does generate the correct code.

 > So is there some real problem here that I missed,
 > or is this to make some code-checking tool that I don't have happy?

Out of curiousity, I thought I'd see what was lurking in a -Wextra
build a while back. It's 99.9% noise, but a lot of it is trivial stuff
like this.

	Dave

-- 
http://www.codemonkey.org.uk

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Fix empty macros in acpi.
  2007-07-03  5:44         ` Dave Jones
@ 2007-07-22  4:55           ` Len Brown
  0 siblings, 0 replies; 7+ messages in thread
From: Len Brown @ 2007-07-22  4:55 UTC (permalink / raw)
  To: Dave Jones, Al Viro, Linux Kernel

On Tuesday 03 July 2007 01:44, Dave Jones wrote:
> On Tue, Jul 03, 2007 at 01:22:47AM -0400, Len Brown wrote:
> 
>  > whelp, it seems that the reason for this patch is this:
>  > 
>  > #define DBG()
>  > 
>  > if(...)
>  > 	DBG();
>  > next_c_statement
>  > 
>  > which turns into
>  > if(...) ;
>  > next_c_statement
>  > 
>  > But since there is an intervening ';', this code is still functionally correct
>  > and a decent compiler will delete the test altogether, yes?
> 
> Right, gcc does generate the correct code.
> 
>  > So is there some real problem here that I missed,
>  > or is this to make some code-checking tool that I don't have happy?
> 
> Out of curiousity, I thought I'd see what was lurking in a -Wextra
> build a while back. It's 99.9% noise, but a lot of it is trivial stuff
> like this.

okay, i'll apply it to reduce the noise.

thanks,
-Len

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2007-07-22  5:01 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-06-12 23:33 Fix empty macros in acpi Dave Jones
2007-06-13  0:00 ` Al Viro
2007-06-13  0:21   ` Dave Jones
2007-06-13  0:26     ` Al Viro
2007-07-03  5:22       ` Len Brown
2007-07-03  5:44         ` Dave Jones
2007-07-22  4:55           ` Len Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).