[Mesa-dev] [Mesa-stable] [PATCH] glsl: Disallow return with a void argument from void functions.

Kenneth Graunke kenneth at whitecape.org
Fri Jul 12 18:44:12 PDT 2013


On 07/12/2013 06:04 PM, Carl Worth wrote:
> Carl Worth <cworth at cworth.org> writes:
>> From: Matt Turner <mattst88 at gmail.com>
>>
>> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>> (cherry picked from commit fcaa48d9cc8937e0ceb59dfd22ef5b6e6fd1a273)
>>
>> Conflicts (resolved by Carl Worth <cworth at cworth.org>):
>> 	src/glsl/ast_to_hir.cpp
>
> [Oops. I meant to pass the --compose option to "git send-email" to add a
> bit of explanation before sending this patch out. Adding that
> explanation here as a reply instead.]
>
> Hi Matt,
>
> Here's a version of your patch as I updated it to apply to the stable
> branch.
>
> It didn't cherry-pick cleanly in the first place since master has since
> added code based on ARB_shading_language_420pack to coerce some return
> types.
>
> The stable branch doesn't add that code, so the context got confused
> when applying this patch. Below is my resolution of the conflict.
>
> My reading is that even without support for shading_language_420pack on
> the stable branch, adding this additional error case makes sense,
> (though perhaps the comment here, which references 420pack looks a
> little out of place, but I'm not concerned about that.)
>
> Please review what I did here, and let me know if you approve or if
> something different should be done, (cherry-picking additional dependent
> patches or dropping this from stable or other).
>
> -Carl

Hey Carl, Matt,

There are two kinds of bug fixes:
1. Patches that make valid programs work
2. Patches that disallow invalid programs

This is in category 2 - it won't fix any applications, but runs a risk 
of breaking working programs.  We often avoid backporting these.

In this case, it's a very small risk, and your backport looks fine, so 
I'm fine with it going to stable.  Either way.

--Ken


More information about the mesa-dev mailing list