Re: [PATCH v4 12/13] tools/xenstore: use generic accounting for remaining quotas

[Juergen Gross <jgross@xxxxxxxx>]

On 03.05.23 16:39, Juergen Gross wrote:
> On 03.05.23 12:18, Julien Grall wrote:
> > On 05/04/2023 08:03, Juergen Gross wrote:
> > > +static void domain_acc_valid_max(struct domain *d, enum accitem what,
> > > +                 unsigned int val)
> > > +{
> > > +    assert(what < ARRAY_SIZE(d->acc));
> > > +    assert(what < ARRAY_SIZE(acc_global_max));
> > > +
> > > +    if (val > d->acc[what].max)
> > > +        d->acc[what].max = val;
> > > +    if (val > acc_global_max[what] && domid_is_unprivileged(d->domid))
> > > +        acc_global_max[what] = val;
> > > +}
> > > +
> > >   static int domain_acc_add_valid(struct domain *d, enum accitem what, int add)
> > >   {
> > >       unsigned int val;
> > > -    assert(what < ARRAY_SIZE(d->acc));
> > I think this assert should be kept because...
> >
> > > -
> > >       if ((add < 0 && -add > d->acc[what].val) ||
> > ... of this check. Otherwise, you would check that 'what' is within the bounds 
> > after the use.
> Okay.
Hmm, I'm no longer sure this is a good reason to duplicate the assert().

Following this reasoning I'd need to put it into even more functions. And an
assert() triggering a little bit late is no real problem, as it will abort
xenstored anyway.

Additionally with the global and the per-domain arrays now covering all
possible quotas, it would even be reasonable to drop the assert()s in
domain_acc_valid_max() completely.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature