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-...@openjdk.java.net; build-...@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


Reply via email to