[OpenJDK 2D-Dev] Remove including of link.h to improve portability
Jonathan Lu
luchsh at linux.vnet.ibm.com
Thu Mar 22 04:59:42 UTC 2012
Verified, thanks a lot!
- Jonathan
2012/3/22 Jonathan Lu <luchsh at linux.vnet.ibm.com>
> Verified
>
> Thanks, Charles!
>
> - Jonathan
>
>
> 2012/3/22 Charles Lee <littlee at linux.vnet.ibm.com>
>
>> On 03/21/2012 04:17 PM, Jonathan Lu wrote:
>>
>>> Hi, the original patch was not rebased to the latest 2d code, here's the
>>> rebased patch.
>>>
>>> http://cr.openjdk.java.net/~**luchsh/7152519_2/jdk.patch<http://cr.openjdk.java.net/%7Eluchsh/7152519_2/jdk.patch>
>>>
>>> Regards!
>>>
>>> - Jonathan
>>>
>>> On 03/21/2012 07:37 AM, Igor Nekrestyanov wrote:
>>>
>>>> Looks fine to me.
>>>>
>>>> -igor
>>>>
>>>> On 3/20/12 3:30 PM, Phil Race wrote:
>>>>
>>>>> I'm (still) OK with this .. one other reviewer please somebody.
>>>>>
>>>>> -phil.
>>>>>
>>>>> On 3/20/2012 1:37 AM, Jonathan Lu wrote:
>>>>>
>>>>>> Hello, could anyone please help to take a look at this splitted patch?
>>>>>>
>>>>>> Thanks!
>>>>>>
>>>>>> -Jonathan
>>>>>>
>>>>>> 2012/3/13 Jonathan Lu <luchsh at linux.vnet.ibm.com <mailto:
>>>>>> luchsh at linux.vnet.ibm.**com <luchsh at linux.vnet.ibm.com>>>
>>>>>>
>>>>>> Hi Phil,
>>>>>>
>>>>>> Thanks a lot for the review and testing, I've splited the patch
>>>>>> into two parts, one for 2D repository and another for TL. Here's
>>>>>> the patch for 2D repository:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~**luchsh/7152519/<http://cr.openjdk.java.net/%7Eluchsh/7152519/>
>>>>>> <http://cr.openjdk.java.net/%**7Eluchsh/7152519/<http://cr.openjdk.java.net/%7Eluchsh/7152519/>
>>>>>> >
>>>>>>
>>>>>> So could anybody please help to do another review?
>>>>>>
>>>>>> Thanks a lot!
>>>>>>
>>>>>> - Jonathan
>>>>>>
>>>>>>
>>>>>> On 03/13/2012 02:22 AM, Phil Race wrote:
>>>>>>
>>>>>> I added two of those includes myself I believe and I doubt I
>>>>>> did it unless needed
>>>>>> and others apparently found it necessary too. So we need to be
>>>>>> sure this is OK.
>>>>>> However at least one of those I added dates back to Solaris 8
>>>>>> being the build platform
>>>>>> so maybe its no longer needed.
>>>>>>
>>>>>> I ran the patch through our internal jprt build system on all
>>>>>> platforms which
>>>>>> for Solaris uses a recent Solaris 10 update and it built fine.
>>>>>> I didn't notice
>>>>>> any new warnings on the files I know anything about.
>>>>>>
>>>>>> > 7152519
>>>>>> It was incorrectly submitted as awt, I moved it to 2D.
>>>>>>
>>>>>> But I think you should split this into 2 patches.
>>>>>> The above bug can be used for all the 2D ones and push to 2d.
>>>>>>
>>>>>> The other one for the nio and security patches can go to the
>>>>>> "tl" repo.
>>>>>>
>>>>>> -phil.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 3/11/2012 8:29 PM, Jonathan Lu wrote:
>>>>>>
>>>>>> Bug 7152519 has been created for this patch.
>>>>>>
>>>>>> - Jonathan
>>>>>>
>>>>>> On 03/09/2012 07:49 PM, Jonathan Lu wrote:
>>>>>>
>>>>>> Hello 2d-dev,
>>>>>>
>>>>>> I find that link.h is included in several place of
>>>>>> OpenJDK code, mostly together with dlfcn.h, but this
>>>>>> caused portability problem in my testing on some Unix
>>>>>> platforms, such as AIX.
>>>>>> So far as I see OpenJDK only makes use of basic
>>>>>> POSIX.1-2001 compatible dynamic library manipulation
>>>>>> functions, such as dlopen, dlclose, dlsym, dlerr
>>>>>> functions, no other extensions found, so is link.h
>>>>>> still neccessary for current implementation? because
>>>>>> link.h is not found in the c-POSIX standard headers
>>>>>> (http://en.wikipedia.org/wiki/**C_POSIX_library<http://en.wikipedia.org/wiki/C_POSIX_library>)
>>>>>> and I
>>>>>> think this removal will be an enhancement for
>>>>>> portability, does that make sense?
>>>>>>
>>>>>> Here's the proposed patch, since most parts of it are
>>>>>> from Java2d so I post it here for discussion.
>>>>>> http://cr.openjdk.java.net/~**luchsh/remove_link_h/<http://cr.openjdk.java.net/%7Eluchsh/remove_link_h/>
>>>>>> <http://cr.openjdk.java.net/%**7Eluchsh/remove_link_h/<http://cr.openjdk.java.net/%7Eluchsh/remove_link_h/>
>>>>>> >
>>>>>> <http://cr.openjdk.java.net/%**7Eluchsh/remove_link_h/<http://cr.openjdk.java.net/%7Eluchsh/remove_link_h/>
>>>>>> >
>>>>>>
>>>>>> And one more question, in
>>>>>> src/solaris/native/sun/java2d/**x11/XRBackendNative.c
>>>>>> I
>>>>>> found following comments
>>>>>> #ifdef __solaris__
>>>>>> /* Solaris 10 will not have these symbols at runtime
>>>>>> */
>>>>>> #include <link.h>
>>>>>>
>>>>>> And in src/solaris/native/sun/awt/**fontpath.c,
>>>>>> #include <dlfcn.h>
>>>>>> #ifndef __linux__ /* i.e. is solaris */
>>>>>> #include <link.h>
>>>>>> #endif
>>>>>>
>>>>>> I've built successfully on Ubuntu 11.10 32bit and
>>>>>> OpenSolaris Express 2010.11 x86, the patch seems to be
>>>>>> OK, but does anybody know the situation on Solaris
>>>>>> (e.g. Solaris 10) of this problem?
>>>>>> I assume it will also comply with POSIX.1-2001
>>>>>> standard, and provide all the required functions in
>>>>>> dlfcn.h, right?
>>>>>>
>>>>>> Cheers!
>>>>>> - Jonathan
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>> Hi Jonathan,
>>
>> The changeset is:
>>
>> Changeset: 604067ec3ced
>> Author: luchsh
>> Date: 2012-03-22 12:47 +0800
>> URL:http://hg.openjdk.java.**net/jdk8/2d/jdk/rev/**604067ec3ced<http://hg.openjdk.java.net/jdk8/2d/jdk/rev/604067ec3ced>
>>
>> 7152519: Dependency on non-POSIX header file<link.h> causes portability
>> problem
>> Reviewed-by: prr, igor
>>
>> ! src/solaris/native/sun/awt/**fontpath.c
>> ! src/solaris/native/sun/java2d/**opengl/OGLFuncs_md.h
>> ! src/solaris/native/sun/java2d/**x11/XRBackendNative.c
>>
>>
>> Please verify it.
>>
>> Thank you all for reviewing.
>>
>> --
>> Yours Charles
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20120322/0a7ab506/attachment.html>
More information about the 2d-dev
mailing list