Re: [Qemu-devel] [PATCH 4/4] pci: use pci_config_header in pci.c

[Gerd Hoffmann <kraxel@redhat.com>]

Anthony Liguori wrote:
> Gerd Hoffmann wrote:
>> This makes use of the new pci_config_header struct in pci.c,
>> squashing a bunch of casts and hard-coded magic numbers.
>>   
> 
> I thought the last bit of feedback that you received from Paul was that
> it seemed like a waste to go through the trouble of introducing this PCI
> config structure but then forcing the users to do explicit endian
> casting.  I agree with this and was expecting the updated patches to do
> the endian conversion automatically.
> 
> Any reasons for not doing it this way?

I'm not sure what you are expecting here.  The idea to introduce
gcc-checkable byteordered types (via struct trick) was rejected.  So I
have le32 & friends just typedef'ed to the corrosponding int*_t types
and use the existing cpu_*() byteorder macros.  The new types are
basically documentation now.

Endian swapping must still be done by the users.  Lots of casts can be
dropped though, which IMHO improves the readability alot (random chunk
picked from patch):

-    cmd = le16_to_cpu(*(uint16_t *)(d->config + PCI_COMMAND));
+    cmd = le16_to_cpu(conf->command);

I'm not aware of any way to do the byteswapping here automatically.

Except maybe introducing accessor functions/macros for each and every
struct element and hide the byteswapping macro call therein.  Which in
turn makes providing the struct to pci drivers sort of pointless ...

cheers,
  Gerd