qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Qemu-block] [PATCH v2 2/4] Introduce attributes to qem


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v2 2/4] Introduce attributes to qemu timer subsystem
Date: Thu, 18 Oct 2018 10:26:21 +0100
User-agent: Mutt/1.10.1 (2018-07-13)

On Wed, Oct 17, 2018 at 01:24:28PM +0200, Paolo Bonzini wrote:
> 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?

Yes, something like that is good.

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]