admin@glassfish.java.net

Re: CODE REVIEW: FindBugs: ASLauncher and DASLauncher

From: Lloyd L Chambers <Lloyd.Chambers_at_Sun.COM>
Date: Tue, 27 Mar 2007 17:31:49 -0700

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
>