RFR(S): JDK-8073584 Native compilation warning in unpack code

[Dmitry Samersoff]

David,

On 2015-02-24 05:23, David Holmes wrote:
> On 24/02/2015 12:02 AM, Dmitry Samersoff wrote:
>> Hi Everyone,
>>
>> Webrev updated in-place (press shift-reload)
>>
>> http://cr.openjdk.java.net/~dsamersoff/JDK-8073584/webrev.01/
> 
> share/native/libunpack/jni.cpp
> 
> 295     return (jobject) NULL;
> 
> Why do you need a cast on NULL?

It's a recommended (from warning-safe point of view) way to deal with
internally defined types because it allows to typedef jobject to
non-pointer type if necessary.

Yes, a bit of paranoia in this case.

>> Updated formatting.
>>
>> Hack in main.cpp replaced with true error check.
> 
> Not sure it is appropriate to lump the "res != 1" in with the CR error.
> Doesn't this case deserve its own u.abort(xxx) ?

I tend to agree with you, but it's out of scope of warning cleanup.

It's important for warning cleanup to don't alter code behavior and with
this fix code behaves exactly the same way as it is today - if read
fails, unpack aborts with "CRC error, invalid compressed data" message.

-Dmitry


> 
> Thanks,
> David
> 
>> -Dmitry
>>
>>
>> On 2015-02-23 05:07, David Holmes wrote:
>>> On 21/02/2015 4:27 AM, Dmitry Samersoff wrote:
>>>> Hi Everyone,
>>>>
>>>> It's my $0.02 to the warning cleanup work. Please review:
>>>>
>>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8073584/webrev.01/
>>>>
>>>> Notes:
>>>>
>>>> I use an ugly trick: (void) (read() + 1) to get rid of ignored value
>>>> warning because since gcc 4.6 just (void) is not enough.
>>>
>>> Why not just check the return value for correctness?
>>>
>>> David
>>>
>>>>
>>>> -Dmitry
>>>>
>>>>
>>
>>


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.