[9] Fix for JDK-8074824: Resolve disabled warnings for libawt_xawt

[Phil Race]

On 07/07/2016 10:25 AM, Semyon Sadetsky wrote:
> On 7/7/2016 7:48 PM, Ajit Ghaisas wrote:
>
>> Thanks Phil for the review.
>> Please find my answers below.
>>
>> Semyon, can you please comment on Phil's question below?
> I agree with this change. The check is not needed.

Ok. so the fix overall seems fine in that case although regarding

 >but it is out of purview of this fix as it may introduce behavioral 
change.

I do not entirely buy it is out of purview I won't push any further on 
it for now
but won't be surprised if someone else reports an unused variable warning.


-phil.

>
> --Semyon
>>
>> Regards,
>> Ajit
>>
>> -----Original Message-----
>> From: Phil Race
>> Sent: Wednesday, July 06, 2016 2:02 AM
>> To: Ajit Ghaisas
>> Cc: Sergey Bylokhov; Erik Joelsson; 2d-dev; awt-dev at openjdk.java.net; 
>> build-dev at openjdk.java.net
>> Subject: Re: [9] Fix for JDK-8074824: Resolve disabled warnings for 
>> libawt_xawt
>>
>> It is not always clear to me what warning is being suppressed and why 
>> you have chosen a particular solution/action
>>
>>
>> this next one looks like it might introduce an unused variable warning.
>> What was it solving ? That the code was not checking a return value ?
>>
>> size_t bytesWritten = write ( AWT_WRITEPIPE, &wakeUp_char, 1 ); 
>> //bytesWritten is unused
>>
>> Isn't the compiler's point here that you *should* be doing something 
>> with the result?
>> Not just ignoring it differently ...
>>
>> ----------------------------------------------
>> [Ajit] : there was a warning of type 'unused-result' for write() 
>> method. Now, I have just assigned that to a variable which fixes the 
>> warning.
>> I think, we should have some code to do error check on bytesWritten 
>> and return it - but it is out of purview of this fix as it may 
>> introduce behavioral change.
>> ----------------------------------------------
>>
>> -----
>>
>> and this one ? I want Semyon to comment on what this code is trying 
>> to do in its original form since it was added for GTK3.
>>
>> @@ -1989,11 +2029,7 @@
>>    static guint8 recode_color(gdouble channel)
>>    {
>>        guint16 result = (guint16)(channel * 65535);
>> -    if (result < 0) {
>> -        result = 0;
>> -    } else if (result > 65535) {
>> -        result = 65535;
>> -    }
>> +
>>        return (guint8)( result >> 8);
>>    }
>>
>> -----------------------------------------------------
>> [Ajit] : there was a warning about guint16 will not be less than 0 
>> and larger than 65535. Hence I have removed code checking this range.
>> -----------------------------------------------------
>>
>> -----
>>
>>
>> -phil.
>>
>> On 06/23/2016 12:09 AM, Ajit Ghaisas wrote:
>>> Hi,
>>>
>>> Bug :
>>>       https://bugs.openjdk.java.net/browse/JDK-8074824
>>>       (Resolve disabled warnings for libawt_xawt)
>>>
>>> As part of fixing this bug, I have -
>>>
>>>       1. Fixed warnings in source code after removing blanket 
>>> warning suppressions from makefile.
>>>
>>>       2. In case the warning fix is not possible, converted blanket 
>>> warning suppression for this library to suppression of warnings for 
>>> individual files.
>>>
>>>       3. Added comments in makefile for the warning suppression that 
>>> cannot be fixed.
>>>
>>>      One type of gcc warning 'deprecated-declarations' will be fixed
>>> separately (as part of JDK-8160146)
>>>
>>>
>>> I have built the changes successfully on all supported platforms.
>>>
>>>
>>> Webrev :
>>>       http://cr.openjdk.java.net/~aghaisas/8074824/webrev.00/
>>>
>>> Request you to review.
>>>
>>> Regards,
>>> Ajit
>