Re: CODE REVIEW: FindBugs: ASLauncher and DASLauncher
go for it...
Lloyd L Chambers wrote:
> 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
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>