[Mesa-dev] [PATCH V2 1/2] mesa: Fix backward compatibility for XML parser

[Nicolai Hähnle]

On 19.08.2017 10:01, QuRyu wrote:
> From: Quentin Liu <Quentin.Liu.0415 at gmail.com>
> 
> If the type of drirc options are changed, the parser will not be able to
>   recognize xml files that had been present before the change. To achieve
> backward compatibility, the parser is relaxed to recognize boolean type
> options with enum values.
> ---
>   src/util/xmlconfig.c | 17 +++++++++++++++--
>   1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/src/util/xmlconfig.c b/src/util/xmlconfig.c
> index d3f47ec..d81a07b 100644
> --- a/src/util/xmlconfig.c
> +++ b/src/util/xmlconfig.c
> @@ -317,8 +317,21 @@ 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_celshalde, were formerly enum
> +             * values. Now that they have been turned into boolean values,
> +             * to achieve backward compatibility relax the check here a
> +             * little bit */
> +            int value = strToI(str...ing, &tail, 0);
> +            if (value == 1)
> +                v->_bool = true;
> +            else if (value == 0)
> +                v->_bool = false;
> +            else
> +                return false; /* wrong value here */
> +            }

Erm... this doesn't even compile.

Please make sure to always do at least this minimal amount of 
verification on patches you send out.

Another thing I noticed is that your patch has trailing whitespace. 
Please remove that (you should be able to configure your editor to do it 
automatically). Actually, Git should warn you about this.

Anyway, apart from these two issues that are really simple to fix, your 
changes look good to me, thanks!

Cheers,
Nicolai

P.S.: It is customary to add a line like

v2: brief explanation of changes

when sending out updated patches. Not really a big deal in this case, 
just thought you should know for the future :)


> +	}
> +
>           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.