[Mesa-dev] [PATCH] glcpp: Check version_resolved in the proper place.

[Carl Worth]

Matt Turner <mattst88 at gmail.com> writes:
> The check was in the wrong place, ...

I don't doubt that the change here is good and correct, but I think
there's likely some additional renaming that can be applied:

> +	if (parser->version_resolved)
> +		return;
> +
>  	parser->version_resolved = true;

Checking this field immediately before setting it clearly looks correct.

>  glcpp_parser_resolve_version(glcpp_parser_t *parser)
>  {
> -	if (parser->version_resolved)
> -		return;
> -

But it looks odd to not be checking the "version_resolved" field within
the "resolve_version" function.

I'm not looking at the actual code now in order to provide a
recommendation, but perhaps it would be more clear if the field were
renamed to be consistent with the naming of the function performing the
guarded manipulation?

-Carl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140125/c868d529/attachment.pgp>