Re: [Qemu-block] [PATCH v2 2/4] Introduce attributes to qemu timer subsystem

[Artem Pisarenko]

Yes, but without words ..."when they fire" in attributes comments.

ср, 17 окт. 2018 г. в 17:24, Paolo Bonzini <address@hidden>:

On 17/10/2018 12:57, Artem Pisarenko wrote:
>> Further down in this patch the notation is QEMU_TIMER_ATTR_<id>, which I
>> think is clearer because QEMU_TIMER_ATTR(id) looks like a (non-existent)
>> macro.  Please use the QEMU_TIMER_ATTR_<id> notation consistently.
>
> Yes, I've just forgot to update comments after previous patch version,
> where it actually was macro.
>
>> What is the purpose of this bit?  I guess it's just here as a
>> placeholder because no real bits have been defined yet.  Hopefully the
>> next patch removes it (/* This placeholder is removed in the next patch
>> */ would be a nice way to document this for reviewers).
>
> It's just to prevent compilation errors, as required by
> https://wiki.qemu.org/Contribute/SubmitAPatch#Split_up_long_patches
>
>> The enum isn't needed and makes debugging harder since the bit number is
>> implicit in the enum ordering.  This alternative is clearer and more
>> concise:
>> 
>>   #define QEMU_TIMER_ATTR_foo BIT(n)
>
> Agree.

Like this?

...

--

С уважением,
  Артем Писаренко