admin@glassfish.java.net

Re: CODE REVIEW: FindBugs: ASLauncher and DASLauncher

From: Byron Nevins <Byron.Nevins_at_Sun.COM>
Date: Tue, 27 Mar 2007 17:52:41 -0700

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
>