Re: [PATCH 1/4] hvmloader: Fixup pci_write* macros

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 15/06/15 17:09, Mihai Donțu wrote:
> On Mon, 15 Jun 2015 16:23:13 +0100 Jan Beulich wrote:
>>>>> On 15.06.15 at 16:35, <andrew.cooper3@citrix.com> wrote:
>>> On 15/06/15 15:30, Don Slutz wrote:
>>>> On 06/15/15 10:19, Andrew Cooper wrote:
>>>>> Furthermore, what about devfn or reg?
>>>>>
>>>> devfn and reg do not need the bracketing since they are just passed,
>>>> but I have no issue with adding the extra brackets.
>>> Macros, under all circumstances, should have all of their parameters
>>> bracketed for safety.
>> No, as this harms readability:
>>
>> #define macro(x) function((x))
>>
>> would completely pointlessly be having an extra set of parentheses.
>> And
>>
>> #define macro(x, y) function(x, y)
>>
>> is just the logical extension to that: There is nothing the expressions
>> passed as arguments could contain to require parenthesization, as
>> there's no operator of precedence lower than comma (and if either
>> argument contains a comma expression, it needs to be
>> parenthesized at the invocation site anyway).
> I think parenthesis are just long term guards against classic surprises
> such as:
>
> #define macro(x) function(x * 2)
> ...
> macro(N + 3)
>
> You could treat this case separately though, but people often go for the
> "good practice".
>

Personally, I would go a step further and suggest that if a macro is not
doing {string,token}isation, or deliberately playing with local state,
it should be a static inline function instead (although I do appear to
be in the minority with this view).

This way, the compiler is able to provide far more coherent error
messages and spot type errors, both of which reduce the number of errors
which can accidentally be introduced.  Furthermore, the compiler is also
able to ignore the 'inline' if it decides it knows better, which
generally results in more efficient code.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel