qemu-devel
[Top][All Lists]
Advanced

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

[Qemu-devel] Re: [PATCH v6 08/12] pcie/aer: helper functions for pcie ae


From: Michael S. Tsirkin
Subject: [Qemu-devel] Re: [PATCH v6 08/12] pcie/aer: helper functions for pcie aer capability
Date: Thu, 21 Oct 2010 10:07:01 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

On Thu, Oct 21, 2010 at 02:15:24PM +0900, Isaku Yamahata wrote:
> Thank you for detailed review.
> 
> On Wed, Oct 20, 2010 at 11:56:16AM +0200, Michael S. Tsirkin wrote:
> > > +static uint32_t aer_log_del(PCIEAERLog *aer_log)
> > > +{
> > > +    uint32_t i = aer_log->consumer;
> > > +    aer_log->consumer = aer_log_next(aer_log->consumer, 
> > > aer_log->log_max);
> > > +    return i;
> > > +}
> > 
> > 
> > Please just use 'int' or 'unsigned' instead of uint32_t if you simply
> > want to say 'a number'.  Using specific length makes it impossible to
> > say where you *really* want a value.
> 
> PCIEAERLog is saved/loaded. So explicit sized number is chosen.

I didn't notice. But consumer/producer are not guest visible, are they?
If yes I think it's a mistake to save/load them, tying us to
a specific implementation: just calculate and save the # of
valid entries in the log.

Also I put the comment here but it is a general one: pls go over the
code, and just take a look at what types you use all over and think
whether size really matters. In most places it does not, it just must be
big enough, so use int or unsigned there.  It will be much harder for
others to do so later.

> -- 
> yamahata



reply via email to

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