Subject: Re: [PATCH 1/2 v8] powerpc: introduce macro spin_event_timeout()

From: Timur Tabi <[email-masked]>
Date: Tue, 26 May 2009 13:17:20 -0500

Geoff Thorpe wrote:

> rc = spin_event_timeout((ret = in_be32(x) & 0x14), ...);

It's an interesting idea, but I have two problems with it:

1) This approach is that it depends on the internals of the macro.  That is, you're sneaking in an assignment in the hopes that the code will behave properly.  You see that the current code evaluates the condition only once, so it works.  The code will look like this:

2) Unlike the wait_event_xxx macros, I believe that the actual evaluated expression is important to the caller.  So 90% of the time, the caller is going to pass in "ret = (condition)", which means it makes more sense to have this as a built-in feature of the macro.

So if you're okay with v9 of the patch, please ACK it.

-- 
Timur Tabi
Linux kernel developer at Freescale