Need reviewer - jdk7 jdwpgen, added netbeans project, fixed findbugs errors
Kelly.Ohair at Sun.COM
Fri Aug 21 09:15:26 PDT 2009
Alan Bateman wrote:
> Kelly O'Hair wrote:
>> Still need reviewer ...
>> 6853636: Fix warnings in jdwpgen, add jdwpgen NetBeans project
>> Pretty harmless code cleanup on the tool used to generate
>> some of the JDWP code for the debugger.
> As you say, these changes are harmless but I'm curious as to what is
> generating the warnings. I assume NetBeans is highlighting the unused
> imports and unused fields but the initialization of fields to their
> default values in AbstractNamedNode, AltNode, and others? Is findbugs
> the reason why you are removing the usage of System.exit? As this is a
> command line tool then I would think its usage is justified. The other
> changes look fine except ConstantSetNode.constantMap - can that be final?
Perhaps. These are a combination of javac -Xlint:all warnings and findbugs
The issue with System.exit() is that use of it prevents the Java code
from ever being used inside say... ant build scripts, or some other
java app in the future. So a clean exit out of main() is best, or throwing an
exception when you have a serious error. It was the System.exit()
calls in Parse.java and Node.java that were the issue, I just tend to
get rid of all uses if at all possible, and it was possible here.
Findbugs warned about the more dangerous places.
On the field, findbugs says:
"Field not initialized in constructor
This field is never initialized within any constructor, and is
therefore could be null after the object is constructed.
This could be a either an error or a questionable design,
since it means a null pointer exception will be generated
if that field is dereferenced before being initialized."
So setting it to null makes it explicit and gets rid of the warning.
I didn't feel it was worth re-doing the code to avoid a null value here.
> The NB project files look fine but just wondering if it is really
> necessary to check in the findbug settings, are there non-default
> settings here?
Probably not necessary, it just so happens that the netbeans findbugs
plugin places it in the none private area and if I don't include it
I'll need to explicitly list the file in .hgignore.
It seemed harmless and small.
More information about the serviceability-dev