Verified, thanks a lot! - Jonathan
2012/3/22 Jonathan Lu <[email protected]> > Verified > > Thanks, Charles! > > - Jonathan > > > 2012/3/22 Charles Lee <[email protected]> > >> 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 <[email protected] <mailto: >>>>>> [email protected].**com <[email protected]>>> >>>>>> >>>>>> 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 >> >> >
