Re: [Xen-devel] [PATCH v1 1/4] xen: add hypercall for getting parameters at runtime

[Vasilis Liaskovitis <vliaskovitis@xxxxxxxx>]

On Mon, 2019-03-18 at 15:02 +0100, Jan Beulich wrote:
> > > > From the return value of strcmp()? I don't think so, because
> > > > you
> may have run past all table entries. Instead it's that property
> that you can use, i.e. checking whether ...
> 
> > > > +        for ( param = start; param < end; param++ )
> 
> ... the loop end condition was hit.

right.

> 
> > > I'm also
> > > unconvinced this is appropriate if the value is actually
> > > a signed quantity.
> > 
> > isn't OPT_UINT only for unsigned integers?
> 
> You'll notice that there's no OPT_INT or OPT_SINT. OPT_UINT
> may be slightly misleading as a name. Otoh we probably don't
> have very many signed integer options.

I see. Assuming an unsigned parameter is problematic then.

Doesn't the .data.param section (between __param_start and __param_end)
only include runtime parameters, of which the integer ones are all
unsigned integers at the moment? Or would you like to see this
functionality eventually used for all parameters, including non-runtime 
as well?

> > 
> > with the new get_func prototype being:
> > 
> >             const char *(*get_func)(void);
> > 
> > returning a string with the current value of the parameter.
> 
> And where would the storage live for the string?

I was thinking these custom functions would return static strings, as
loglvl_str() does, and these would eventually be copied into the user
provided values[] buffer via snprintf in get_params().

> 
> > e.g. the function for loglvl, guest_loglvl could return output
> > using
> > loglvl_str() :  "Nothing", "Errors", "Errors and warnings" etc.
> > 
> > An alternative prototype could be:
> > 
> >             int (*get_func)(char *output);
> > 
> > if we want the function to write the current parameter value into a
> > caller-provided buffer, and possibly return error codes.
> 
> And how would the callee know how much space there is?

yes, an extra size argument would be needed from the caller here.

Let me know if either of these or a different approach is preferred.

> > 
> You certainly shouldn't access them from sysctl.c. You should
> simply make get_params() access them instead of what its
> only caller passes, at which point the function should be
> renamed to it only caller's name.

... of course.


thanks,

- Vasilis


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