Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-22 Thread Alan Bateman
Xueming Shen wrote: Alan, After staring those simple, 11 lines of change for minutes, I believe we should simply go back with the original approach at http://cr.openjdk.java.net/~sherman/6858865/webrev.00 The change in http://cr.openjdk.java.net/~sherman/6858865/webrev.00 obviously is

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-22 Thread Xueming Shen
On 11/22/2010 05:52, Alan Bateman wrote: Xueming Shen wrote: Alan, After staring those simple, 11 lines of change for minutes, I believe we should simply go back with the original approach at http://cr.openjdk.java.net/~sherman/6858865/webrev.00 The change in

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-22 Thread Alan Bateman
Xueming Shen wrote: : We can not simply return 0 if the out buffer length is NOT 0 even when in buffer length is 0. The de/inflater might have something to push/flush out even there is no more input. Consider the buffer in last invocation of de/inflating is not big enough, de/infalte() gets

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-19 Thread Alan Bateman
Xueming Shen wrote: Alan, It might not be a real regression if only consider the supported platforms (and yes, the malloc manpageI can found does clearly indicate NULL is for error). However I prefer to add some checks to make sure it behaves the same (compared to before the #6728376 change

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-19 Thread Xueming Shen
On 11/19/2010 01:31, Alan Bateman wrote: Xueming Shen wrote: Alan, It might not be a real regression if only consider the supported platforms (and yes, the malloc manpageI can found does clearly indicate NULL is for error). However I prefer to add some checks to make sure it behaves the

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-19 Thread Alan Bateman
Xueming Shen wrote: : We can probably do that for Inflater.c (and probably better do that at java level before we even come down here), but thing gets a little complicated for Deflater. One of the paths of the deflateBytes is to deal with parameter(s) change, so if malloc does not fail for

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-19 Thread Xueming Shen
On 11/19/2010 09:55 AM, Alan Bateman wrote: Xueming Shen wrote: : We can probably do that for Inflater.c (and probably better do that at java level before we even come down here), but thing gets a little complicated for Deflater. One of the paths of the deflateBytes is to deal with

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-19 Thread Alan Bateman
Xueming Shen wrote: : I might not explain it clear enough. For Deflater.c we want to at least keep the malloc path for if ((*env)-GetBooleanField(env, this, setParamsID)) { } so it will continue go down to deflateParams() to reset the params, with a valid non-NULL ptr

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-19 Thread Xueming Shen
On 11/19/2010 01:33 PM, Alan Bateman wrote: Xueming Shen wrote: : I might not explain it clear enough. For Deflater.c we want to at least keep the malloc path for if ((*env)-GetBooleanField(env, this, setParamsID)) { } so it will continue go down to deflateParams() to

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-19 Thread Xueming Shen
On 11/19/2010 01:51 PM, Alan Bateman wrote: Xueming Shen wrote: : Why do you want to feed the zlib with a null buffer? :-) So are you saying that if the length is 0 that it still looks at the buffer? If so, then I'm okay with the last webrev. zlib does buffer null check before anything

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-19 Thread Xueming Shen
Alan, After staring those simple, 11 lines of change for minutes, I believe we should simply go back with the original approach at http://cr.openjdk.java.net/~sherman/6858865/webrev.00 The change in http://cr.openjdk.java.net/~sherman/6858865/webrev.00 obviously is problematic, especially