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/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 parameter(s) change, so if 
>> malloc does not
>> fail for 0-length request (true on all supported platforms), the 
>> deflateBytes goes down
>> to zlib's deflateParams() and if there is nothing left to flush, 
>> "this" invocation can successfully
>> re-set the param(s) and return 0. The reason why I decided to go with 
>> the "safe and simple"
>> solution last night, the proposed webrev at least should behave the 
>> exactly the same as it
>> was before.
>>
>> -Sherman
>>
> I think it would be a bit cleaner for inflater.  For deflater would it 
> cleaner to just skip the mallocs when the lengths are 0 (for the 
> deflateParams path).


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 on most platforms, even for 0-length buf, this is
what it was before #6728376 and what it is now, We don't what to have
another "regression" (change the params, invoke the deflate() with 0-length
buffer, but now it returns 0 with change the setting, I have to invoke it
again with...) here. And with the "len != 0" check, it will be no behavior
change as well for that weird OS.

For other 2 paths (the Inflater.c and the
    (*env)->GetBooleanField(env, this, setParamsID) == false
path, I believe/think/guess it should be safe to bypath the zlib by
checking len/in_len and returning 0 if any of them is zero. But this is
again a "change", probably:-) harmless giving my understand/reading of
zlib code and the existing regression tests.

So the "bit cleaner" webrev is
http://cr.openjdk.java.net/~sherman/6858865/webrev/

But it deal with the 0-length issue "differently" at different places.
This is why I proposed last night the "safe" and "simple: change.

Anyway, I'm fine with the "bit-cleaner" approach, so if you're OK with
it, throw it in:-)

The "old" webrev is at
http://cr.openjdk.java.net/~sherman/6858865/webrev.00/

-Sherman