[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