Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

On 04/19/2012 04:05 PM, Sean Chou wrote: Thanks David and Alan, shall I find some one to commit it ? On Thu, Apr 19, 2012 at 8:53 AM, David Holmeswrote: On 19/04/2012 4:05 AM, Alan Bateman wrote: On 18/04/2012 14:02, David Holmes wrote: On 18/04/2012 10:23 PM, Sean Chou wrote: Hi David,

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

On 19/04/2012 09:05, Sean Chou wrote: Thanks David and Alan, shall I find some one to commit it ? I assume Charles or Neil will push it for you. If they aren't available then I should be able to find someone to sponsor this. -Alan.

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

Thanks David and Alan, shall I find some one to commit it ? On Thu, Apr 19, 2012 at 8:53 AM, David Holmes wrote: > On 19/04/2012 4:05 AM, Alan Bateman wrote: > >> On 18/04/2012 14:02, David Holmes wrote: >> >>> On 18/04/2012 10:23 PM, Sean Chou wrote: >>> Hi David, Alan, So is the

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

On 19/04/2012 4:05 AM, Alan Bateman wrote: On 18/04/2012 14:02, David Holmes wrote: On 18/04/2012 10:23 PM, Sean Chou wrote: Hi David, Alan, So is the patch acceptable ? There is still the matter of the unexpected NULL if strdup fails. I'd need to see the clients for this code to see how the

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

On 18/04/2012 14:02, David Holmes wrote: On 18/04/2012 10:23 PM, Sean Chou wrote: Hi David, Alan, So is the patch acceptable ? There is still the matter of the unexpected NULL if strdup fails. I'd need to see the clients for this code to see how they handle failure. My concern is the c

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

On 18/04/2012 10:23 PM, Sean Chou wrote: Hi David, Alan, So is the patch acceptable ? There is still the matter of the unexpected NULL if strdup fails. I'd need to see the clients for this code to see how they handle failure. My concern is the case where the caller sees a NULL return wh

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

Hi David, Alan, So is the patch acceptable ? It's webrev: http://cr.openjdk.java.net/~zhouyx/7159982/webrev.02/ On Wed, Apr 18, 2012 at 12:15 PM, David Holmes wrote: > On 18/04/2012 1:15 PM, Sean Chou wrote: > >> Hi David, >> >> The current implementation uses static char array to keep

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

On 18/04/2012 1:15 PM, Sean Chou wrote: Hi David, The current implementation uses static char array to keep the error message, so it is possible when two errors happen at the same time, the error message will be modified. I have a testcase attached in http://mail.openjdk.java.net/pipermail/

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

Hi David, The current implementation uses static char array to keep the error message, so it is possible when two errors happen at the same time, the error message will be modified. I have a testcase attached in http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-April/009766.html .

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

Hi Sean, On 18/04/2012 12:37 PM, Sean Chou wrote: To free the error string in ZIP_Open is a result of discussion with hotspot. They said the error string is never used and they do not want to do the free work in hotspot for ZIP_Open... Ok. I assume there are no other callers of this metho

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

Hi David, To free the error string in ZIP_Open is a result of discussion with hotspot. They said the error string is never used and they do not want to do the free work in hotspot for ZIP_Open... strdup would cause a NULL error string if memory allocation is failed. If strdup is not used,

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

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 NU

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

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 wrote: > On 12/04/2012 06:40, Sean Chou wrote: > >> Hi Alan, >> >>Many thanks. >> >>I updated

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

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/

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

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/ Please take a look once more. On Thu, Apr 12, 2012 at 4:09 AM, Alan Bateman wrote: > On 11/04/2012

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

On 11/04/2012 15:58, Sean Chou wrote: Hi hotspot guys, Would any one like to take a look at this? I'm trying to fix a potential race in ZIP_Open, it is found classLoader.cpp uses this function. So a webrev for hotspot is made as well, but I need a sponsor from hotspot as suggested by Alan

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

Hi hotspot guys, Would any one like to take a look at this? I'm trying to fix a potential race in ZIP_Open, it is found classLoader.cpp uses this function. So a webrev for hotspot is made as well, but I need a sponsor from hotspot as suggested by Alan Bateman. The start of this thread is

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

On 10/04/2012 08:10, Sean Chou wrote: And it is found that hotspot calls ZIP_Open through (*ZipOpen) in file classLoader.cpp .So I also made a patch for it and add hotspot-dev to cc list. File classLoader.cpp is the only one I have found calling ZIP_Open. The webrev for hotspot is: h

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

would like to suggest to replace the static error string in >> ZIP_Put_In_Cache0 with on stack memory. >> >> : >> >> >> The modification is here: >> http://cr.openjdk.java.net/~**zhouyx/OJDK-482/webrev.00/<http://cr.openjdk.java.net/~zhouyx/OJDK-482/webrev

Re: Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

On 09/04/2012 08:53, Sean Chou wrote: Hi all, I would like to suggest to replace the static error string in ZIP_Put_In_Cache0 with on stack memory. : The modification is here: http://cr.openjdk.java.net/~zhouyx/OJDK-482/webrev.00/ . Good catch, this one had probably been there

Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

Hi all, I would like to suggest to replace the static error string in ZIP_Put_In_Cache0 with on stack memory. Because it takes more time to modify a string than an int typed error number, it opens a window larger than int typed error number and might bring in race in multi-thread user cases