You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by sebb <se...@gmail.com> on 2008/05/02 21:02:48 UTC

[jxpath] comments

Eclipse finds a few potential NPEs:

The variable p1 may be null at this location	
commons-jxpath/src/java/org/apache/commons/jxpath/ri/model	NodePointer.java	line
797
The variable parameters may be null at this
location	commons-jxpath/src/java/org/apache/commons/jxpath/util	MethodLookupUtils.java	line
53
The variable parameters may be null at this
location	commons-jxpath/src/java/org/apache/commons/jxpath/util	MethodLookupUtils.java	line
121
The variable parameters may be null at this
location	commons-jxpath/src/java/org/apache/commons/jxpath/util	MethodLookupUtils.java	line
282

[XPathParser has lots of unnecessary semi-colons]

[One or two unused imports]


Findbugs says (amongst other things):

Code may fail to close
stream	commons-jxpath/src/java/org/apache/commons/jxpath	JXPathContextFactory.java	line
192

equals() methods in the CollectionPointer and NullElementPointer
classes both use instanceof - this is likely to cause assymetric
behaviour.

Dead store to active1 in
org.apache.commons.jxpath.ri.parser.XPathParserTokenManager.jjMoveStringLiteralDfa15_0(long,
long, long, long)	line 1107:

if (((active0 &= old0) | (active1 &= old1)) == 0L)

should that be as follows:?

if (((active0 & old0) | (active1 & old1)) == 0L)

Incorrect lazy initialization and update of static field
org.apache.commons.jxpath.ri.JXPathContextReferenceImpl.nodeFactoryArray
in
createNodeFactoryArray() at line 119
The static field is written and then the contents are updated; it
would be safer to finish updating the contents before writing the
field.
Might also need synchronisation ...

The variable parent cannot be null at this
location	commons-jxpath/src/java/org/apache/commons/jxpath/ri/axes	ChildContext.java	line
113

There are quite a few non-transient non-serializable fields, but
perhaps the classes are really intended for serialising.

Also various methods in SimpleCharStream and XPathParser,
XPathParserTokenManager don't start with a lower-case letter.
Probably too late to do anything about that now...

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [jxpath] comments

Posted by Emmanuel Bourg <eb...@apache.org>.
Michiel Kalkman a écrit :
> True, on the other hand, just for my curiosity:
> 1. shouldn't the javaCC project be notified through some bug report ?
> 2. shouldn't there be an apache commons bug report which purpose is to
> keep track of the bug reported against the JavaCC project ?

Well, that may be worth suggesting an optimization to the JavaCC project 
for removing the unused code, but personally I don't feel it's that 
important to track this change if ever it happens.

Emmanuel Bourg


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [jxpath] comments

Posted by Michiel Kalkman <mi...@gmail.com>.
True, on the other hand, just for my curiosity:
1. shouldn't the javaCC project be notified through some bug report ?
2. shouldn't there be an apache commons bug report which purpose is to
keep track of the bug reported against the JavaCC project ?

Just wondering ...

Regards,
Michiel

On 5/2/08, Emmanuel Bourg <eb...@apache.org> wrote:
> sebb a écrit :
>
> > Dead store to active1 in
> >
> org.apache.commons.jxpath.ri.parser.XPathParserTokenManager.jjMoveStringLiteralDfa15_0(long,
> > long, long, long)       line 1107:
> >
> >
>
> >
> > Also various methods in SimpleCharStream and XPathParser,
> > XPathParserTokenManager don't start with a lower-case letter.
> > Probably too late to do anything about that now...
> >
>
> The classes in the parser package are generated by JavaCC, they should be
> excluded from the code analysis.
>
> Emmanuel Bourg
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [jxpath] comments

Posted by Emmanuel Bourg <eb...@apache.org>.
sebb a écrit :

> Dead store to active1 in
> org.apache.commons.jxpath.ri.parser.XPathParserTokenManager.jjMoveStringLiteralDfa15_0(long,
> long, long, long)	line 1107:
> 

> 
> Also various methods in SimpleCharStream and XPathParser,
> XPathParserTokenManager don't start with a lower-case letter.
> Probably too late to do anything about that now...

The classes in the parser package are generated by JavaCC, they should 
be excluded from the code analysis.

Emmanuel Bourg


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org