admin@glassfish.java.net

Re: CODE REVIEW: FindBugs: ASLauncher and DASLauncher

From: Lloyd L Chambers <Lloyd.Chambers_at_Sun.COM>
Date: Thu, 29 Mar 2007 10:34:34 -0700

Thanks Byron. Would they be OK to commit then?

Lloyd

On Mar 27, 2007, at 5:52 PM, Byron Nevins wrote:

> I have reconsidered my initial knee-jerk reaction based on reading
> the changes carefully and realizing that it was YOU that made the
> changes. :-)
>
> I approve.
>
>
> Lloyd L Chambers wrote:
>> Byron,
>>
>> Well, I don't agree at all, but I don't need the hassle of
>> convincing others either. I haven't made any changes to the
>> System.properties handling and the change is very straightforward.
>>
>> Lloyd
>>
>> On Mar 27, 2007, at 5:13 PM, Byron Nevins wrote:
>>
>>> My immediate reaction is: NO! Don't commit this change. It is
>>> too dangerous.
>>>
>>> It's interesting that Findbugs is very concerned about writing to
>>> *one* class variable from an instance, but it doesn't mind
>>> writing to the "Super-Global-Statics" -- System.Properties, which
>>> this code does. System Properties are hyper-thread-unsafe.
>>>
>>> If you disagree then I need the new source file itself and ~ 4
>>> hours to thoroughly test it. IMHO, it isn't worth it...
>>>
>>>
>>>
>>> Lloyd L Chambers wrote:
>>>> Please review no later than noon on Thursday 3/30.
>>>>
>>>> Fixes a FindBugs complaint of "Write to static field".
>>>>
>>>> Lloyd
>>>>
>>>>
>>>> RCS file: /cvs/glassfish/admin/servermgmt/src/java/com/sun/
>>>> enterprise/admin/servermgmt/launch/ASLauncher.java,v
>>>> retrieving revision 1.12
>>>> diff -w -u -r1.12 ASLauncher.java
>>>> --- admin/servermgmt/src/java/com/sun/enterprise/admin/
>>>> servermgmt/launch/ASLauncher.java 16 Mar 2007 19:54:22
>>>> -0000 1.12
>>>> +++ admin/servermgmt/src/java/com/sun/enterprise/admin/
>>>> servermgmt/launch/ASLauncher.java 27 Mar 2007 17:46:55 -0000
>>>> @@ -154,7 +154,7 @@
>>>> private static final String DOMAIN_NAME_SYSTEM_PROPERTY =
>>>> "domain.name";
>>>> private static final String SCRIPT_PATH_SYSTEM_PROPERTY =
>>>> "com.sun.aas.scriptpath";
>>>>
>>>> - static int returnValue = 1;
>>>> + int _returnValue = 1;
>>>> String[] securityInfo; // username & passwords if
>>>> neccessary...
>>>>
>>>> // WBN the level is here as a constant so that it can be
>>>> changed to INFO or better
>>>> @@ -176,9 +176,12 @@
>>>>
>>>> public static void main(String[] args)
>>>> {
>>>> + int returnValue = 1;
>>>> +
>>>> + ASLauncher pl=null;
>>>> try
>>>> {
>>>> - ASLauncher pl=new ASLauncher();
>>>> + pl=new ASLauncher();
>>>> pl.preProcess(args); // figure out relocatable
>>>> stuff
>>>> pl.getSecurityFromUser(args);
>>>> Process process = pl.process(args, pl.securityInfo);
>>>> @@ -187,9 +190,21 @@
>>>> {
>>>> e.printStackTrace();
>>>> }
>>>> + finally
>>>> + {
>>>> + if ( pl != null )
>>>> + {
>>>> + returnValue = pl.getReturnValue();
>>>> + }
>>>> + }
>>>> +
>>>> System.exit(returnValue);
>>>> }
>>>>
>>>> + int getReturnValue()
>>>> + {
>>>> + return _returnValue;
>>>> + }
>>>>
>>>> /**
>>>> * This method is meant to be called from the
>>>> PLBootstrap.class to finish the setup
>>>> @@ -335,7 +350,7 @@
>>>> Process process = null;
>>>> try
>>>> {
>>>> - returnValue=1;
>>>> + _returnValue=1;
>>>> securityInfo = SecurityInfo;
>>>>
>>>> if(securityInfo != null)
>>>> @@ -379,7 +394,7 @@
>>>> if (bDebug) System.out.println("ASLauncher
>>>> Executing command ..");
>>>>
>>>> // if no exception, should have executed properly
>>>> or logged errors
>>>> - returnValue=0;
>>>> + _returnValue=0;
>>>>
>>>> if(externalLogfileHandler != null)
>>>> {
>>>>
>>>>
>>>> RCS file: /cvs/glassfish/admin/servermgmt/src/java/com/sun/
>>>> enterprise/admin/servermgmt/launch/DASLauncher.java,v
>>>> retrieving revision 1.1
>>>> diff -w -u -r1.1 DASLauncher.java
>>>> --- admin/servermgmt/src/java/com/sun/enterprise/admin/
>>>> servermgmt/launch/DASLauncher.java 18 Oct 2006 05:29:53
>>>> -0000 1.1
>>>> +++ admin/servermgmt/src/java/com/sun/enterprise/admin/
>>>> servermgmt/launch/DASLauncher.java 27 Mar 2007 17:46:55 -0000
>>>> @@ -35,7 +35,7 @@
>>>> info.setSystemProps();
>>>> asLauncher = new ASLauncher();
>>>> asLauncher.process(args);
>>>> - return asLauncher.returnValue;
>>>> + return asLauncher._returnValue;
>>>> }
>>>>
>>>>
>>>> -------------------------------------------------------------------
>>>> --
>>>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>>>> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>>>>
>>>
>>> --------------------------------------------------------------------
>>> -
>>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>>> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>