Re: [Qemu-devel] [PATCH v6] s390x/pci: add common function measurement block

[Thomas Huth <thuth@redhat.com>]

On 2019-01-07 10:08, Pierre Morel wrote:
> On 04/01/2019 15:39, Cornelia Huck wrote:
>> On Thu,  3 Jan 2019 11:17:00 +0100
>> Pierre Morel <pmorel@linux.ibm.com> wrote:
>>
>>> +static void fmb_update(void *opaque)
>>> +{
>>> +    S390PCIBusDevice *pbdev = opaque;
>>> +    int64_t t = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
>>> +    int i;
>>> +
>>> +    /* Update U bit */
>>> +    pbdev->fmb.last_update *= 2;
>>> +    pbdev->fmb.last_update |= UPDATE_U_BIT;
>>> +    if (fmb_do_update(pbdev, offsetof(ZpciFmb, last_update),
>>> +                      pbdev->fmb.last_update,
>>> +                      sizeof(((ZpciFmb *)0)->last_update))) {
>>
>> I really don't want to be a pain, but... this looks weird. Why not
>> simply sizeof(pbdev->fmb.last_update)?
> 
> Isn't it preferable to use the size of the destination?

But "(ZpciFmb *)0" in the context of QEMU isn't the destination either,
it's an invalid pointer. So I think I agree with Cornelia,
"sizeof(pbdev->fmb.last_update)" looks much easier to read and
understand here.

 Thomas