Re: [PATCH 01/11] tools/libs/store: add get- and set-quota related functions

["Anthony PERARD" <anthony.perard@xxxxxxxxxx>]

On Mon, Mar 16, 2026 at 08:51:21AM +0100, Jürgen Groß wrote:
> On 13.03.26 15:23, Anthony PERARD wrote:
> > On Thu, Mar 05, 2026 at 02:51:58PM +0100, Juergen Gross wrote:
> > > diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c
> > > index 8f4b90a3cf..dda37f7526 100644
> > > --- a/tools/libs/store/xs.c
> > > +++ b/tools/libs/store/xs.c
> > > @@ -1456,6 +1456,117 @@ bool xs_set_features_domain(struct xs_handle *h, 
> > > unsigned int domid,
> > >           return xs_bool(xs_talkv(h, iov, ARRAY_SIZE(iov), NULL));
> > >   }
> > > +char **xs_get_quota_names(struct xs_handle *h, unsigned int *num)
> > > +{
> > > + struct xsd_sockmsg msg = { .type = XS_GET_QUOTA };
> > > + struct iovec iov[1];
> > > + char **quota;
> > > + char *reply;
> > > + char *c;
> > > + unsigned int i;
> > > +
> > > + iov[0].iov_base = &msg;
> > > + iov[0].iov_len  = sizeof(msg);
> > > +
> > > + reply = xs_talkv(h, iov, ARRAY_SIZE(iov), NULL);
> > > + if (!reply)
> > > +         return NULL;
> > > +
> > > + *num = 1;
> > > + for (c = reply; *c; c++)
> > > +         if (*c == ' ')
> > > +                 (*num)++;
> > > +
> > > + quota = malloc(*num * sizeof(char *) + strlen(reply) + 1);
> > > + c = (char *)(quota + *num);
> > > + strcpy(c, reply);
> > > + for (i = 0; i < *num; i++) {
> > > +         quota[i] = c;
> > > +         c = strchr(c, ' ');
> > > +         if (c) {
> >
> > If `c` is NULL, it's likely that this is the last iteration of the `for`
> > loop. But just in case, should we prevent the code from doing another
> > round and prevent `strchr(NULL, ' ')`? (Or just check that `c` is !NULL,
> > and let the loop finish set NULL for the remaining slot in `quota`)
>
> Not sure this is really needed. *num is set just a few lines further up
> using the same way to count the number of strings. Do we really need to do
> consistency checks of intermediate results in such a short function?

I guess it's ok.

> >
> > > +                 *c = 0;
> > > +                 c++;
> > > +         }
> > > + }
> > > +
> > > + return quota;
> > > +}
> > > +
> > > +bool xs_get_global_quota(struct xs_handle *h, char *quota,
> > > +                  unsigned int *value)
> > > +{
> > > + struct xsd_sockmsg msg = { .type = XS_GET_QUOTA };
> > > + struct iovec iov[2];
> > > +
> > > + iov[0].iov_base = &msg;
> > > + iov[0].iov_len  = sizeof(msg);
> > > + iov[1].iov_base = quota;
> > > + iov[1].iov_len  = strlen(quota) + 1;
> > > +
> > > + return xs_uint(xs_talkv(h, iov, ARRAY_SIZE(iov), NULL), value);
> > > +}
> > > +
> > > +bool xs_set_global_quota(struct xs_handle *h, char *quota,
> > > +                  unsigned int value)
> > > +{
> > > + struct xsd_sockmsg msg = { .type = XS_SET_QUOTA };
> > > + char val_str[MAX_STRLEN(value)];
> >
> > MAX_STRLEN doesn't have a great name, I wounder what is was :-). And
> > it's not about a maximum size of payload that could go on xs wire or
> > something, it's actually the maximum string size that can take a
> > numerical value, when converted to charaters.
>
> Unfortunately the MAX_STRLEN() macro is defined in a public header file.
> I could define another macro with a different name doing the same and
> use that here, but MAX_STRLEN() would still be there.
>
> What is your preference?

Well, it's probably better to leave it like that. Having two different
macro names might be more confusing than useful. And I see that the
macro is only defined three time in the repo. Next time I'll stumble on
MAX_STRLEN, I'll know what it mean.

Cheers,,


--
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech