Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

[David Holmes]

Certainly the string management in this code is a bit of a mess, but I 
don't understand why the strdup's of string literals have been 
introduced. Even if for a good reason this seems to imply that an 
allocation failure will result in a NULL where before the caller was 
guaranteed never to get NULL in the error case, and that could lead to SEGV.

Also with the change to avoid changes on the hotspot side, the actual 
cause of the open failure has been lost in ZIP_Open

David

On 13/04/2012 1:14 AM, Sean Chou wrote:
> Hi Alan,
>
>      I made a new webrev, added the comments and the 2 other modification.
> It's now : http://cr.openjdk.java.net/~zhouyx/7159982/webrev.02/
>
> On Thu, Apr 12, 2012 at 4:24 PM, Alan Bateman<Alan.Bateman at oracle.com>wrote:
>
>> On 12/04/2012 06:40, Sean Chou wrote:
>>
>>> Hi Alan,
>>>
>>>     Many thanks.
>>>
>>>     I updated the patch, ZIP_Open frees the error message and set "Zip
>>> file open error".
>>>
>>> The new webrev is : http://cr.openjdk.java.net/~**
>>> zhouyx/7159982/webrev.01/<http://cr.openjdk.java.net/%7Ezhouyx/7159982/webrev.01/><
>>> http://cr.openjdk.java.net/%**7Ezhouyx/7159982/webrev.01/<http://cr.openjdk.java.net/%7Ezhouyx/7159982/webrev.01/>
>>>>
>>>
>>>
>>>     Please take a look once more.
>>>
>> This looks much better. I think we'll need to add comments to the ZIP_*
>> functions so that it's clear to anyone using them when they need to free
>> the error message and then they don't.
>>
>> One implementation nit at zip_util.c L876 where it should check if pmsg is
>> NULL and I think the tests should be reversed so that its:
>>
>> if (file != NULL&&  pmsg != NULL&&  *pmsg != NULL)  { ... }
>>
>> One other minor nit is L875 where there is a space on either side of the
>> "*", best to keep the style consistent.
>>
>> -Alan.
>>
>
>
>