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

[James Melvin]

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
>