Fix empty macros in acpi.

Fix empty macros in acpi.

[Dave Jones]

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

Re: Fix empty macros in acpi.

[Al Viro]

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.

Re: Fix empty macros in acpi.

[Dave Jones]

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

Re: Fix empty macros in acpi.

[Al Viro]

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.

Re: Fix empty macros in acpi.

[Len Brown]

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

Re: Fix empty macros in acpi.

[Dave Jones]

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

Re: Fix empty macros in acpi.

[Len Brown]

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