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

Re: [Xen-devel] [PATCH v5 2/2] xen/arm: consolidate make_timer_node and make_timer_domU_node



On Fri, Aug 2, 2019 at 12:41 PM Julien Grall <julien.grall@xxxxxxx> wrote:
> >
> > Well, I address each of the comments or write about it explicitly in
> > other cases.
> > In this particular case, I'd added  '-1', but later did not merge it
> > due to mistake.
> > So it supposed to be the next:
> > +    unsigned int irq[MAX_TIMER_PPI-1]
>
> Please no '-1', it is worst than hardcoding value. In the code you are using 
> an
> element of an enum to access the array. There are no guarantee the last 
> element
> is actually the one you want to drop and therefore you risk to overflow it if
> mistakenly used.
>
I agree that using -1 is not the best idea. It would be better to
introduce a new enum for that. However, since we already have the enum
with 4 items for that, it is better to use it as is.

> The risk is not worth compare to saving just 4-byte on the stack.

Completely agree about it, so I will use MAX_TIMER_PPI(as it is done
now) in the next patch series version,

Thanks

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.