RFR (S): 7125793: MAC: test_gamma should always work

[Staffan Larsen]

Looks great!

On 23 jan 2012, at 02:43, James Melvin wrote:

> Hi Staffan,
> 
> Comments inline...
> 
> On 1/22/12 6:43 AM, Staffan Larsen wrote:
>> Inline.
>> 
>> On 21 jan 2012, at 16:01, James Melvin wrote:
>>>> 
>>>> java_md.c:GetJVMPath - I don't think the code inside "#ifndef GAMMA"
>>>> needs to be changed. The code is always compiled with GAMMA defined. The
>>>> "#ifndef GAMMA" is just there to facilitate comparison with the original
>>>> code in the JDK.
>>> 
>>> Correct. The code in the jdk has also changed to something similar, but
>>> with ifdefs instead of code logic. I'm not sure of the history behind
>>> the gamma launcher and how much effort has been expended to keep the jdk
>>> and hotspot launchers in sync in these sections. There are additional
>>> changes proposed to the jdk launcher. They are in review at the moment.
>>> I'd like to keep the current code in place and revise later to reflect
>>> the jdk changes when the dust settles.
>>> 
>> 
>> I think the last update was about a year ago, when I rebased the
>> launcher code in hotspot to 6u22 code. At that point, I believe the only
>> changes to the code was inside "#ifdef GAMMA".
> 
> OK, I agree.  I've reverted the code in the #ifndef GAMMA section.  We
> can rebase after the JDK launcher changes.  No problem.
> 
> 
>>>> os_bsd.cpp:2580 - (nit) incorrect indentation
>>> 
>>> Hmmm... This must be a webrev/browser thing. I'm looking in xemacs and
>>> the webrev via Firefox on Mac and I don't see an indentation problem.
>>> I'll send you the source file in a private email to inspect. Let me know
>>> if you still see the problem.
>> 
>> Could be a tabsize issue. I think the line was preceded by a tab instead
>> of spaces.
> 
> Indeed, there was a tab at the beginning of the line.  Hg jcheck would
> have discovered it after commit.  Thanks!
> 
> New webrev and JPRT available, including builds on Mac OS X...
> 
> WEBREV:  http://cr.openjdk.java.net/~jmelvin/7125793/webrev.04
> JPRT: http://prt-web.us.oracle.com//archive/2012/01/2012-01-22-205611.jmelvin.7125793/
> 
> Thanks,
> 
> Jim
> 
>> Thanks,
>> /Staffan
>>