From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH 1/4] hvmloader: Fixup pci_write* macros
Date: Mon, 15 Jun 2015 15:35:51 +0100
Message-ID: <557EE2C7.6060801@citrix.com>
References: <1434377752-15705-1-git-send-email-dslutz@verizon.com>
	<1434377752-15705-2-git-send-email-dslutz@verizon.com>
	<557EDEE0.9060901@citrix.com> <557EE198.1080303@one.verizon.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <557EE198.1080303@one.verizon.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Don Slutz <dslutz@verizon.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
Cc: Keir Fraser <keir@xen.org>, Ian Campbell <ian.campbell@citrix.com>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, Ian Jackson <ian.jackson@eu.citrix.com>, Don Slutz <don.slutz@gmail.com>, Jan Beulich <jbeulich@suse.com>, Wei Liu <wei.liu2@citrix.com>
List-Id: xen-devel@lists.xenproject.org

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