Yasumasa, Looks good for me.
-Dmitry On 2016-05-17 17:31, Yasumasa Suenaga wrote: > Hi Dmitry, > > Thank you for your comment. > I've fixed them in new webrev. Could you review again? > > http://cr.openjdk.java.net/~ysuenaga/JDK-8155936/webrev.01/ > > > Yasumasa > > > On 2016/05/17 19:25, Dmitry Samersoff wrote: >> Yasumasa, >> >> 1. Please use strcasecmp for true/false >> >> 2. You may save one strcmp call by replacing it to >> (*arg == '0' && *(arg+1) == 0) >> >> -Dmitry >> >> >> On 2016-05-17 13:15, Yasumasa Suenaga wrote: >>> PING: Could you review it? >>> We need a reviewer. >>> >>>> bug id: https://bugs.openjdk.java.net/browse/JDK-8155936 >>>> webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8155936/webrev.00/ >>> >>> >>> Thanks, >>> >>> Yasumasa >>> >>> >>> On 2016/05/06 1:18, Gerard Ziemski wrote: >>>> I’m including serviceability mailing list. >>>> >>>> bug id: https://bugs.openjdk.java.net/browse/JDK-8155936v >>>> webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8155936/webrev.00/ >>>> >>>> >>>> cheers >>>> >>>>> Begin forwarded message: >>>>> >>>>> From: Yasumasa Suenaga <[email protected]> >>>>> Subject: Re: RFR: JDK-8155936: Boolean value should be set 1/0 or >>>>> true/false via VM.set_flag jcmd >>>>> Date: May 4, 2016 at 9:34:15 AM CDT >>>>> To: "[email protected]" >>>>> <[email protected]> >>>>> Cc: Gerard Ziemski <[email protected]> >>>>> >>>>> Hi all, >>>>> >>>>> We still need a second Reviewer. >>>>> Could you review? >>>>> >>>>> >>>>> Thanks, >>>>> >>>>> Yasumasa >>>>> >>>>> >>>>> On 2016/05/04 9:32, Yasumasa Suenaga wrote: >>>>>> Hi Gerard, >>>>>> >>>>>>> Reviewed and I will sponsor it. >>>>>> >>>>>> Thanks! >>>>>> >>>>>>> Just one question: there is no existing JDK issue covering this >>>>>>> yet, is there? Can you file one please if none exists yet? >>>>>> >>>>>> I do not change in jdk repos. >>>>>> My change affects jinfo, however jtreg test for jinfo passed. >>>>>> >>>>>> I ran jtreg with two directories: >>>>>> >>>>>> - hotspot/test/serviceability/dcmd/vm >>>>>> - jdk/test/sun/tools/jinfo >>>>>> >>>>>> They work fine. >>>>>> (JInfoRunningProcessFlagTest is failed. But it is listed in >>>>>> ProblemList.) >>>>>> >>>>>> >>>>>> Thanks, >>>>>> >>>>>> Yasumasa >>>>>> >>>>>> >>>>>> On 2016/05/04 3:18, Gerard Ziemski wrote: >>>>>>> hi Yasumasa, >>>>>>> >>>>>>> Thank you for the fix, I like it - the very first time I tried >>>>>>> using jcmd to set a boolean value I tried using “true”, so this >>>>>>> will make jcmd easier to use. >>>>>>> >>>>>>> Reviewed and I will sponsor it. >>>>>>> >>>>>>> Just one question: there is no existing JDK issue covering this >>>>>>> yet, is there? Can you file one please if none exists yet? >>>>>>> >>>>>>> >>>>>>> >>>>>>> cheers >>>>>>> >>>>>>>> On May 3, 2016, at 9:22 AM, Yasumasa Suenaga <[email protected]> >>>>>>>> wrote: >>>>>>>> >>>>>>>> Hi all, >>>>>>>> >>>>>>>> This review request relates to [1]. >>>>>>>> >>>>>>>> We can change a part of -XX option via VM.set_flag jcmd. >>>>>>>> This jcmd requires 1 or 0 as boolean value. >>>>>>>> However, we can set 0 or not (NOT 1). >>>>>>>> >>>>>>>> In jinfo, we can set boolean value with 1/0 or +/-. >>>>>>>> So I think it is useful if VM.set_flag accept boolean value in >>>>>>>> true/false. >>>>>>>> >>>>>>>> I uploaded a webrev for this issue. >>>>>>>> Could you review it? >>>>>>>> >>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8155936/webrev.00/ >>>>>>>> >>>>>>>> I cannot access JPRT. >>>>>>>> So I need a sponsor. >>>>>>>> >>>>>>>> >>>>>>>> Thanks, >>>>>>>> >>>>>>>> Yasumasa >>>>>>>> >>>>>>>> >>>>>>>> [1] >>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-April/019192.html >>>>>>>> >>>>>>>> >>>>>>> >>>> >> >> -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
