Re: [Qemu-devel] [PATCH 1/2] qapi: add visitor for parsing int[KMGT] input string

[Eduardo Habkost]

On Fri, Dec 07, 2012 at 07:57:35PM +0100, Andreas Färber wrote:
[...]
> > +static void parse_type_unit_suffixed_int(Visitor *v, int64_t *obj,
> > +                                         const char *name, const int unit,
> > +                                         Error **errp)
> > +{
> > +    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> > +    char *endp = (char *) siv->string;
> > +    long long val = 0;
> > +
> > +    if (siv->string) {
> > +        val = strtosz_suffix_unit(siv->string, &endp,
> > +                             STRTOSZ_DEFSUFFIX_B, unit);
> > +    }
> > +    if (!siv->string || val == -1 || *endp) {
> > +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, name,
> > +              "a value representable as a non-negative int64");
> 
> Weird indentation remaining, looks as if we could align with errp within
> 80 chars.
> 
> However, I wonder if "unit" is the (physically etc.) correct term here?
> Isn't the "unit" Hz / byte / ... and 1000 more of a conversion factor or
> something? At least that's the way I've seen unit used in the API of
> another project, passing an enum of Hertz, gram, meter/second, etc.

I also think that calling it "unit" is confusing. Strictly speaking, the
"unit" we're dealing with is Hz (and the visitor simply don't care about
unit, it only looks for "unit prefixes").

Can't we call the 1000/1024 parameter "base" or "factor", instead?

-- 
Eduardo