Ian Jackson wrote:
> Andre Przywara writes ("[Xen-devel] [PATCH 2/5] libxl: add xlu_cfg_get_type function"):
>> As cpuid= can be used with two syntaxes (one as a list, the other as a
>> string), we need to distinguish them without an error message. Introduce
>> a helper function to detect the type of entry used before issuing a warning.
> 
> Thanks.  This is a generally good idea although I'm not quite
> convinced by this:
> 
>  +    errno = 0;
>  +    l = strtol(set->values[0], &endptr, 0);
>  +    if (errno == EINVAL || endptr == set->values[0])
>  +        return XLU_CFG_STRING;
>  +    return XLU_CFG_LONG;
> 
> Firstly, it will fail for unsigned longs bigger than LONG_MAX, and we
> would normally think about unsigned longs.
But there is only xlu_cfg_get_long, which returns signed values (used 28 
times in xl_cmdimpl.c). I don't see any usage of strtoul in xl_cmdimpl.c 
which is preceded by xlu_cfg_get_string().

>  Secondly, if callers say things like
>   if (type == XLU_CFG_STRING) ....
> they'll have a bug.
> I would suggest XLU_CFG_ATOM.  Callers can use strto[u]l (or whatever)
> themselves if they need to distinguish numbers from strings.
Makes sense. Do you mean like the attached delta patch?

I could also live with making the reporting of the error in 
libxl_cfg_get_list() optional, so that users aren't bothered with a 
confusing error output everytime. That would make the whole function 
obsolete.

Tell me what you like more.


Regards,
Andre.


-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12