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

[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
>> http://cr.openjdk.java.net/~sherman/6858865/webrev.00
>>
>> obviously is problematic, especially in case like in_len == 0 but 
>> out_len != 0 (no more,
>> or no more new input, but with a valid output buffer), it's 
>> definitely possible the
>> inflater/deflater might have more bites to (and should) output in 
>> this scenario, it's a
>> bug to return 0 here.
>>
>> Sure it's possible to go further like
>>
>> if (in_len == 0)
>>     return 0;
>> ....
>>
>> if (len == 0)
>>     ...
>>
>> But given the purpose of this fix is to solve that particular 
>> "regression" (which actually
>> does not cause any regression for "mainstream" platforms), I prefer 
>> not take the risk
>> of causing another real regression/behavior change here, especially 
>> we got burned
>> couple times here in the past when tried to do better.
>>
>> Agree?
>>
>> -Sherman
> I understand the concern but I don't think that patch-1 is risky. That 
> is, for Inflater, I can't see any side effects to simply returning 0 
> if either length is 0. Same thing for Deflater when not in the code 
> path that calls deflateParams. For the deflateParams cases then the 
> changes in the latest patch is fine with me.
>
>


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 invoked the second time without any 
new data (and the data for the last invocation
has been consumed by the zlib de/inflate). The patch-1 (the second one) 
is simply wrong.

It is probably OK to return 0 if the output buffer is 0, I said 
"probably" is because I would need to read all those
zlib code again to make sure there is no any internal status change (for 
example in case like the "in" buffer is NOT
zero but the "out" buffer is 0, consider the de/inflater might try to 
consume more data even the output buffer is
zero) or even  if there is an internal status change it would not have 
any negative impact. This is why I'm saying
it is probably not worth the effect.


-Sherman



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/core-libs-dev/attachments/20101122/f554b9c4/attachment.html>