On 17.08.2017 04:20, QuRyu wrote:
After changing the type of drirc values, the parser will be unable to
recognize xml files before the change. To achieve backward compatbility,
Spelling: compatibility
The commit message is good, though I would say "... will be unable to
recognize xml files *from* before the change...". I believe that makes
it a bit easier to read.
Some more comments about the code itself inline below.
the parser is relaxed to recognize boolean type options with enum values.
---
src/util/xmlconfig.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/util/xmlconfig.c b/src/util/xmlconfig.c
index d3f47ec..2999f24 100644
--- a/src/util/xmlconfig.c
+++ b/src/util/xmlconfig.c
@@ -317,8 +317,26 @@ parseValue(driOptionValue *v, driOptionType type, const
XML_Char *string)
v->_bool = true;
tail = string + 4;
}
- else
- return false;
+ else {
+ /** Some drirc options, such as pp_shalde, were formerly enum
values.
+ * Now that they have been turned into boolean values, to achieve
+ * backward compatbility relax the check here a little bit */
Good comment, but it should start with a plain /*, i.e. no double-stars.
The double-stars are used for Doxygen comments only (they can be used to
generate documentation of functions, variables, etc.).
Also, fix the spelling of pp_celshade and compatibility.
+ XML_Char *start = string;
+ int value = strToI(string, &tail, 0);
+ if (tail == start) {
+ /* no enum value found */
+ string = start;
+ return false;
+ } else {
+ if (value == 1)
+ v->_bool = true;
+ else if (value == 0)
+ v->_bool = false;
+ else
+ return false; /* wrong value here */
+ }
I believe you can (and therefore should ;-)) simplify this piece of
code, because there is code after the switch statement which does all
the required error checking. Basically, condensing the code to something
like:
int value = strToI(string, &tail, 0);
if (value == 1)
v->_bool = true;
else if (value == 0)
v->_bool = false;
else
return false;
should be enough. All the other error conditions, like empty string, or
garbage after the tail, are handled below the switch statement.
Cheers,
Nicolai
+ }
+
break;
case DRI_ENUM: /* enum is just a special integer */
case DRI_INT:
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
