[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 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).


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