From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:42705)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <anthony.perard@citrix.com>) id 1SWoaD-0005pW-DR
	for qemu-devel@nongnu.org; Tue, 22 May 2012 08:53:02 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <anthony.perard@citrix.com>) id 1SWoa7-0007FX-RL
	for qemu-devel@nongnu.org; Tue, 22 May 2012 08:53:00 -0400
Received: from smtp02.citrix.com ([66.165.176.63]:19937)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <anthony.perard@citrix.com>) id 1SWoa7-0007Es-Kl
	for qemu-devel@nongnu.org; Tue, 22 May 2012 08:52:55 -0400
Message-ID: <4FBB8BF9.2040306@citrix.com>
Date: Tue, 22 May 2012 13:52:09 +0100
From: Anthony PERARD <anthony.perard@citrix.com>
MIME-Version: 1.0
References: <1333467163-25795-1-git-send-email-anthony.perard@citrix.com>
	<1333467163-25795-4-git-send-email-anthony.perard@citrix.com>
	<20120516112445.GB21609@andromeda.dapyr.net>
In-Reply-To: <20120516112445.GB21609@andromeda.dapyr.net>
Content-Type: text/plain; charset="ISO-8859-1"; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [Xen-devel] [PATCH V11 3/8] Introduce
 XenHostPCIDevice to access a pci device on the host.
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Konrad Rzeszutek Wilk <konrad@darnok.org>
Cc: Xen Devel <xen-devel@lists.xensource.com>, QEMU-devel <qemu-devel@nongnu.org>, Stefano Stabellini <Stefano.Stabellini@eu.citrix.com>, "Michael S . Tsirkin" <mst@redhat.com>

On 16/05/12 12:24, Konrad Rzeszutek Wilk wrote:
> On Tue, Apr 03, 2012 at 04:32:38PM +0100, Anthony PERARD wrote:
>> Signed-off-by: Anthony PERARD<anthony.perard@citrix.com>
>
> Looks good, thought I've just couple of tiny comments:
>
>> +#define XEN_HOST_PCI_RESSOURCE_BUFFER_SIZE 512
>
> You might want a comment explaining why 512, and not
> a more precise number like 741.

Actually, I only need the first 7 line of the resources file. So this 
is 399 bytes. I just add a little bit more.

>> +{
> .. snip..
>> +    do {
>> +        rc = read(fd,&buf, sizeof (buf) - 1);
>> +        if (rc<  0&&  errno != EINTR) {
>> +            rc = -errno;
>> +            goto out;
>> +        }
>> +    } while (rc<  0);
>
> Ok, you read it in. Maybe my 'wc' magic is gone, but this
> is what I get:
> [root@localhost 0000:00:02.0]# cat resource | wc -c
> 741
> .. snip..
>> +#define XEN_HOST_PCI_GET_VALUE_BUFFER_SIZE 42
>
> The answer to the life? Can you provide a comment explaining
> the reason why it is 42, please?

I just define more than needed, and 42 is a good number :-).
But the maximum I need is probably 7 (for example '0x8086\n', for a 
vendor id). Or 20, number of digit in LONG_MAX, base 10.

>> +static int xen_host_pci_get_value(XenHostPCIDevice *d, const char *name,
>> +                                  unsigned int *pvalue, int base)
>> +{


-- 
Anthony PERARD