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/>
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.