[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 4/4] cleanup: Change datatype of net->stp to boolean



On 04/12/2013 05:21 AM, Osier Yang wrote:
> ---
>  src/conf/network_conf.c | 2 +-
>  src/conf/network_conf.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index 968cf11..75584a0 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -1748,7 +1748,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
>      /* Parse bridge information */
>      def->bridge = virXPathString("string(./bridge[1]/@name)", ctxt);
>      stp = virXPathString("string(./bridge[1]/@stp)", ctxt);
> -    def->stp = (stp && STREQ(stp, "off")) ? 0 : 1;
> +    def->stp = (stp && STREQ(stp, "off")) ? false : true;

Your conversion is fine, but this does highlight that the code here
interprets *everything* except "off" as "on" (including "OFF", "no",
"Off", "0").

There are actually many similar instances in the parser code. Should we
continue to be so silently strict? Or complain any time the string isn't
*exactly* "on" or "off"? Or make it extremely tolerant about what is
entered (0, off, Off, no, false all mean the same thing)?

Is it worth making a helper function that takes a const char * and
returns true/false according to whatever rules we decide, then use that
everywhere there is an on/off (or yes/no) attribute?



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]