<AWT Dev> RFR: 8154269: Remove unused or unnecessary Xm/Xt files and header includes

[Phil Race]

On 04/15/2016 08:41 AM, Sergey Bylokhov wrote:
> Looks fine then, but mixing of int, jint and short in one method looks 
> suspicious.

A little, which is why I said "(as)" fine.

I've double-checked and Intrinsic.h has :
typedef short           Position;   /* Offset from 0 coordinate         */


And the "int tmpx, tmpy" are populated using XTranslateCoordinates().
At an API level that says "int" but the X protocol is expressly "INT16"
and I don't think the Xserver will return a value that would cause
anything near an overflow.

BTW I just noticed this :

1090     short x1=0, y1=0, x2=0, y2=0;
....

1108     x1 = -(x1 + tmpx);
1109     y1 = -(y1 + tmpy);


That seems unnecessarily general but it is harmless.

-phil.


> On 15.04.16 18:07, Philip Race wrote:
>> I believe Position was a typedef of short so this should be (as) fine.
>>
>> -phil.
>>
>> On 4/15/16, 7:16 AM, Sergey Bylokhov wrote:
>>> Hi, Phil.
>>> Should the "Position" in X11SurfaceData.c:X11SD_ClipToRoot() be
>>> replaced by int(or jint)?
>>>
>>> On 15.04.16 0:17, Phil Race wrote:
>>>> I think it makes sense to remove the typedef of Pixel and directly use
>>>> unsigned long since the sole reference to Pixel is in order to prepare
>>>> values to pass to XSetForeground which expects "unsigned long".
>>>>
>>>> Updated webrev : http://cr.openjdk.java.net/~prr/8154269.1/
>>>>
>>>> Boolean is used in about 22 places. over half in Xwindow.c
>>>> Probably all of these should all be converted to use "Bool" which
>>>> is the Xlib.h definition and indeed often it is used in
>>>> conjunction with "True" and "False" which are defined alongside Bool :
>>>> #define Bool int
>>>> #define Status int
>>>> #define True 1
>>>> #define False 0
>>>>
>>>> The only potential issue is that this is "int" and Boolean was "char"
>>>> but since True and False are ints anyway ..
>>>>
>>>> I also see TRUE and FALSE used which appear like they may come from
>>>> src/java.desktop/share/native/common/awt/debug/debug_util.h
>>>> but since they are the same that is harmless.
>>>>
>>>> So a follow-on bug could just delete the line
>>>> typedef char Boolean
>>>> and replace uses of Boolean with Bool.
>>>>
>>>> phil.
>>>>
>>>> On 04/14/2016 12:28 PM, Phil Race wrote:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8154269
>>>>> http://cr.openjdk.java.net/~prr/8154269/
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8047931
>>>>> listed a number of X11 and medialib files that are unused in the 
>>>>> build.
>>>>> In looking at the VDrawingArea ones it lead to realising
>>>>> we still include Intrinsic.h (an Xt header) in awt.h even though
>>>>> we do not use Xt since the Motif toolkit was removed.
>>>>> So I think we can remove the dependency on those header files.
>>>>> Doing this lead in turn to finding we have at least one unused
>>>>> field, one unused #define and one unused method declaration
>>>>> all referencing Xt types.
>>>>>
>>>>> So I decided to separate out the removal of Xt headers and files 
>>>>> in to
>>>>> this separate
>>>>> bug report and make the other bug exclusively about medialib.
>>>>>
>>>>> The fix adds into awt.h a couple of X11 includes that were implicit
>>>>> from
>>>>> previously including Intrinsic.h and I explicitly added typedefs for
>>>>> Boolean
>>>>> and Pixel which we were referencing in our code since those came from
>>>>> Intrinsic.h
>>>>> I could perhaps have just changed the one usage of Pixel to "unsigned
>>>>> long"
>>>>> but Boolean was used more widely.
>>>>>
>>>>> JPRT has passed builds on all platforms and JDK still seems to work
>>>>> fine on Linux ..
>>>>>
>>>>> One other note : awt_InputMethod.c has a number of places where it
>>>>> still mentions OSX. I didn't attempt to clean that up here as I 
>>>>> suspect
>>>>> it would muddy the patch.
>>>>>
>>>>>   93 #if defined(__linux__) || defined(MACOSX)
>>>>>
>>>>>
>>>>>
>>>>> -phil.
>>>>>
>>>>>
>>>>
>>>
>>>
>
>