[Mesa-dev] [PATCH V2 3/8] glsl: Add array specifier to ast code

Timothy Arceri t_arceri at yahoo.com.au
Tue Jan 21 14:54:24 PST 2014


Hi Paul

Thanks for the thorough review and taking the time to explain how to make improvements. I can already see a number of improvements that can be made to patch 5 based on the suggestions you have already made so you could hold off reviewing that one if you havent started already and I've post a new series in a couple of days.


>>    if (base == NULL)
>>       return glsl_type::error_type;

>>
>I think the only way base could ever be NULL here is if there is a bug 
elsewhere in Mesa--does that seem correct to >you?  IMHO it would be fine to drop the NULL check entirely.

Yes that seems correct. I just got a little nervous about removing existing checks like this as I saw a comment in unrelated code saying something along the lines of "There was once a bug that cause this to be NULL ..." but as you say an assert would be much better if this is to remain at all. 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140121/8d08d755/attachment.html>


More information about the mesa-dev mailing list