RFR: 7155300 Include pthread.h on all POSIX platforms except Solaris to improve portability

Shi Jun Zhang zhangshj at linux.vnet.ibm.com
Thu Mar 22 04:55:50 UTC 2012


On 3/22/2012 12:36 PM, Charles Lee wrote:
> On 03/21/2012 10:47 AM, Shi Jun Zhang wrote:
>> On 3/12/2012 11:28 AM, Shi Jun Zhang wrote:
>>> On 3/9/2012 6:05 PM, David Holmes wrote:
>>>> On 9/03/2012 7:04 PM, Alan Bateman wrote:
>>>>> On 09/03/2012 08:01, Shi Jun Zhang wrote:
>>>>>> The situation in NativeThread.c is more complicated than other 2
>>>>>> files. I'm not familiar with BSD or Mac. It seems that we don't need
>>>>>> to signal threads on BSD or Mac. And INTERRUPT_SIGNAL on AIX will
>>>>>> definitely be different from the one on Linux. I think we'd better
>>>>>> separate the changes in NativeThread.c from this patch and try to
>>>>>> solve it later.
>>>>> Right, if signals are required there is likely to be differences 
>>>>> across
>>>>> platforms. It is also likely that this code will need to be 
>>>>> changed for
>>>>> Mac too as there are a couple of preemptive close issues to sort out
>>>>> (for file operations, sockets are okay).
>>>>>
>>>>>>>
>>>>>>> The change to socket_md.c looks okay to me but you will need to
>>>>>>> re-base your patch due to the Mac port in jdk8/tl.
>>>>>> I'm a new comer and i got known from Charles about the difference
>>>>>> between jdk8 and jdk8/tl. The latest webrev is based on jdk8/tl.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~zhangshj/pthread/webrev.01/
>>>>> The changes in this webrev look okay to me.
>>>>
>>>> In java_md.c
>>>>
>>>> 1445       /* See above. Continue in current thread if thr_create() 
>>>> failed */
>>>>
>>>> The "see above" is now a "see below".
>>>>
>>>> I think the launcher changes are okay because BSD/OSX won't use 
>>>> this file.
>>>>
>>>> I think the socket changes are okay as long as BSD builds and OSX 
>>>> builds define _ALLBSD_SOURCE. I still don't fully understand if a 
>>>> BSD build and an OSX build are distinct.
>>>>
>>>> David
>>>>
>>>>> -Alan.
>>>>
>>> The comment "see above" has been changed to "see below".
>>>
>>> http://cr.openjdk.java.net/~zhangshj/pthread/webrev.02/
>>>
>> Hi Alan/David,
>>
>> There is no response on this thread for long time. I created a sun 
>> bug 7155300, could you help to review it?
>>
>> The webrev link is 
>> http://cr.openjdk.java.net/~zhangshj/pthread/webrev.02/
>>
> Hi Chance,
>
> Here is the changeset. Please verify it.
>
> Changeset: 1d418ec212ea
> Author:    zhangshj
> Date:      2012-03-22 12:30 +0800
> URL:http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1d418ec212ea
>
> 7155300: Include pthread.h on all POSIX platforms except Solaris to 
> improve portability
> Reviewed-by: alanb, dholmes
>
> ! src/solaris/bin/java_md.c
> ! src/solaris/transport/socket/socket_md.c
>
>
> Thank you all for reviewing.
>
Looks good. Thanks all for reviewing and committing.

-- 
Regards,

Shi Jun Zhang





More information about the core-libs-dev mailing list