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

[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 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
>
The updated webrev looks better, but for the deflateParam path then 
maybe you could do something like this:

if (this_len == 0) {
    in_buf = NULL;
} else {
    in_buf = (jbyte*) malloc(this_len);
    if (in_buf == NULL) {
        JNU_ThrowOutOfMemoryError(env, 0);
        return 0;
    }
}

and same thing for out_buf.

Would that be a bit cleaner?

-Alan