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 <pmo...@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
