Re: [PATCH 11/26] hw/intc/arm_gicv3_its: Use 1ULL when shifting by (DTE.SIZE + 1)

[Peter Maydell]

On Sun, 12 Dec 2021 at 20:43, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 12/11/21 11:11 AM, Peter Maydell wrote:
> >
> >       if (dte_valid) {
> > -        max_eventid = 1UL << (FIELD_EX64(dte, DTE, SIZE) + 1);
> > +        max_eventid = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1);
>
> Without changing the type of max_eventid, I think it'd be easiest to fix the 
> off-by-one
> bug by not changing the comparisions, but changing this computation.  E.g.
>
>    max_eventid = (2 << FIELD_EX64(dte, DTE, SIZE)) - 1;
>
> so that the value becomes UINT32_MAX for SIZE=31.

I think I'd prefer to use a uint64_t. I think that part of the reason
for all these off-by-one errors is a lack of consistency in how we
chose to name variables and whether we put in them the max-allowed
value or the 2^n value, so the series tries to standardize on
"always call it num_thingy and always use the 2^n value". I prefer
to keep the consistency rather than rearrange things in this
one case so it can use a uint32_t.

-- PMM