Re: [Xen-devel] [PATCH v5 22/24] libxlu: introduce new APIs

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Feb 13, 2015 at 02:12:29PM +0000, Ian Jackson wrote:
> Wei Liu writes ("[PATCH v5 22/24] libxlu: introduce new APIs"):
> > These APIs can be used to manipulate XLU_ConfigValue and XLU_ConfigList.
> > 
> > +    if (value->type != XLU_STRING) {
> > +        if (!dont_warn)
> > +            fprintf(cfg->report, "warning: value is not a string\n");
> > +        *value_r = NULL;
> > +        return EINVAL;
> 
> This message needs to include the file and line number, or it is very
> hard for the user to use.  The other call sites (which are based on
> `find') require the caller to provide a name, which means that the
> setting name can be printed too.  Maybe you could do something
> similar.
> 

It is a bit different from a setting because a value doesn't have a
name.

I've added another patch to record line and column number for a value so
that we can use them here.

> If you were feeling keen you could replace these formulaic things with
> something like:
>    return report_bad_cfg(dont_warn, cfg, set, n, "value is not a string");
> or
>    return REPORT_BAD_CFG("value is not a string");
> (being a function or macro which always returns EINVAL), or some such.
> 

Do feel very keen about this since the format string differs from
functions. And it's only one printf anyway.

Wei.

> Thanks,
> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel