[Mesa-dev] [PATCH 1/5] Mesa: Change save_attrib_data() to return boolean

[Brian Paul]

On 12/12/2013 02:19 AM, Juha-Pekka Heikkilä wrote:
> On Thu, Dec 12, 2013 at 3:14 AM, Brian Paul <brianp at vmware.com> wrote:
>> On 12/11/2013 02:05 AM, Juha-Pekka Heikkila wrote:
>>>
>>> Change save_attrib_data() to return true/false depending on success.
>>>
>>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
>>
>>
>> I'd like to reconsider these changes from scratch.
>>
>> The basic issue is just null-checking of the malloc calls in
>> glPush[Client]Attrib().  Why can't we just do something like the (partial)
>> patch below?
>>
>
> The patch below is really similar to my original try at this. Ian
> pointed out from my first try if save_attrib_data would fail we'd lose
> "attr" thus I now have evolved the patch to the way it is.

save_attrib_data() could free the attr data if the 
malloc(gl_attrib_node) fails.  But that's pretty unlikely anyway since 
it's such a small allocation.


> I think now
> it would not in any case leak anything. The original patch for this
> issue with later comments was here:
> https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/archives/mesa-dev/2013-December/049628.html&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=lGQMzzTgII0I7jefp2FHq7WtZ%2BTLs8wadB%2BiIj9xpBY%3D%0A&m=zqSh7DDzSfIFtJicUG1X47XPh1zbuQpq8eL5erODtM8%3D%0A&s=fe0885203b966f88a03e9669530943c79908cd339f961ae50137a400f4145fd9
>
> As a small bonus using helper in glPushAttrib() make the function more
> clean looking and easier to follow.

OK, if you really want to got that way, I don't have a strong opinion. 
But I do have more review comments then...

-Brian