Re: [PATCH RFC] virtio-pci: new config layout: using memory BAR

[Laszlo Ersek <lersek@redhat.com>]

On 05/28/13 19:43, Paolo Bonzini wrote:
> Il 28/05/2013 19:32, Michael S. Tsirkin ha scritto:
>>>>>> +
>>>>>> +    switch (addr) {
>>>>>> +    case offsetof(struct virtio_pci_common_cfg, device_feature_select):
>>>>>> +        return proxy->device_feature_select;
>>>>
>>>> Oh dear no...  Please use defines like the rest of QEMU.
>> Any good reason not to use offsetof?
> 
> I'm not sure it's portable to use it in case labels.  IIRC, the
> definition ((int)&(((T *)0)->field)) is not a valid C integer constant
> expression.  Laszlo?

As long as we use the offsetof() that comes with the C implementation
(ie. we don't hack it up ourselves), we should be safe; ISO C99
elegantly defines offsetof() in "7.17 Common definitions <stddef.h>" p3
as

  The macros are [...]

    offsetof(type, member-designator)

  which expands to an integer constant expression that has type
  *size_t*, the value of which is the offset in bytes, to the structure
  member (designated by /member-designator/), from the beginning of its
  structure (designated by /type/). The type and member designator shall
  be such that given

    static type t;

  then the expression &(t.member-designator) evaluates to an address
  constant. (If the specified member is a bit-field, the behavior is
  undefined.)

("Address constant" is described in 6.6p9 but quoting even that here
doesn't seem necesary.)

From 6.8.4.2p3, "The expression of each case label shall be an integer
constant expression [...]".

So,
(a) if we use the standard offsetof(),
(b) and you can also write, for example,

  static struct virtio_pci_common_cfg t;
  static void *p = &t.device_feature_select;

then the construct seems valid.

(Consistently with the above mention of UB if the specified member is a
bit-field: the address-of operator's constraints also forbid a bit-field
object as operand.)

Laszlo