Re: [OpenJDK 2D-Dev] [9] Fix for JDK-8074824: Resolve disabled warnings for libawt_xawt
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, _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
Re: [OpenJDK 2D-Dev] [9] Fix for JDK-8074824: Resolve disabled warnings for libawt_xawt
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.
--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, _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
Re: [OpenJDK 2D-Dev] [9] Fix for JDK-8074824: Resolve disabled warnings for libawt_xawt
Thanks Phil for the review.
Please find my answers below.
Semyon, can you please comment on Phil's question below?
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, _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
Re: [OpenJDK 2D-Dev] [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, _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 ...
-
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);
}
-
-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
Re: [OpenJDK 2D-Dev] [9] Fix for JDK-8074824: Resolve disabled warnings for libawt_xawt
Hello, I'm happy with the makefile changes, unless anyone else could come up with a solution for any of the remaining warnings. /Erik On 2016-06-23 09:09, 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
[OpenJDK 2D-Dev] [9] Fix for JDK-8074824: Resolve disabled warnings for libawt_xawt
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
