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

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

On 15/06/15 15:30, Don Slutz wrote:
> On 06/15/15 10:19, Andrew Cooper wrote:
>> On 15/06/15 15:15, Don Slutz wrote:
>>> Signed-off-by: Don Slutz <dslutz@verizon.com>
>>> CC: Don Slutz <dslutz@verizon.com>
>> Fix how?  It looks like you are bracketing val.
>>
> If val is an expression, the macro most likely does the wrong thing.
>
> For example:
>
> pci_writeb(devfn, PCI_IO_BASE, addr >> PCI_IO_SHIFT);
>
>
> is pci_write(devfn, reg, 1, (uint8_t)addr >> PCI_IO_SHIFT); which is not
> correct.  I would expect:
>
> pci_write(devfn, reg, 1, (uint8_t)(addr >> PCI_IO_SHIFT));
>
>> This is an improvement, but please always be specific as to what is
>> being fixed.
>>
> Does:
>
> Improve pci_write* macros to act like a function does.
>
> scan better?
>
>> 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.

Simply "Fix pci_write* macros by bracketing their parameters" would be
ok.  The why is obvious to C programmers, but simply "fix" is too
generic to be useful as a description.

~Andrew