Re: [libvirt] [PATCH 4/4] cleanup: Change datatype of net->stp to boolean
On 12/04/13 23:17, Eric Blake wrote:
On 04/12/2013 08:38 AM, Laine Stump wrote:
> On 04/12/2013 05:21 AM, Osier Yang wrote:
>> ---
> 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)?
It also begs the question - is our .rng schema capable of parsing the
same set of options as our C code accepts, and flagging the typos?
As far as I saw from the parsing code, the answer is "no", applies for
most of the parser.
> 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?
Yes, I think having a common parse helper function, as well as using a
common <define> throughout the RNG schema that accepts the same things
as the C code, would go a long way to making this friendlier to users.
Agreed, this can reduce much redundant code.
I also think that adding a way to canonicalize an XML snippet,
including
the option to reject it if it fails the RNG schema, would be a helpful
addition to the API (we've mentioned the idea in the past, but it has
not been implemented yet).
It was on my plate, feel sorry for I didn't finish it, though once raised
up the proposal. Anyway, I was planning to pick it up again recently.
Except the API, validating the XML when define the objects is nice to
have too (though I'm a bit worried about the regressions, as the RNG
schema could also have bugs).
Osier