Hi Azeez,
Find answers, in-line.
In addition all skipped rules are listed in [1]. Let me know if
there are any invalid cases.
Thanks
AmilaJ
[1]
https://svn.wso2.org/repos/wso2/trunk/carbon/core/org.wso2.carbon.user.core/user-core-findbugs-excluded.xml
Afkham Azeez wrote:
>
>
>
>
> On Fri, Sep 24, 2010 at 6:35 PM, Amila Jayasekara <[email protected]
> <mailto:[email protected]>> wrote:
>
> Hi Azeez,
> Sorry for the late response. I was involved in some other
> urgent task.
> I fixed many find bugs issues in user.core and there were several
> which
> i exclude. I would like to verify whether excluded rules are
> reasonable.
>
> 1. OBL_UNSATISFIED_OBLIGATION - Method may fail to clean up stream or
> resource.
> This occurs each time user core tries to do database transaction.
> Sample code,
>
> try {
> prepStmt =
> dbConnection.prepareStatement(DBConstants.ADD_PERMISSION_SQL); <----
> Complaining that this resource is not cleaned up
> ...
> ...
> } catch (SQLException e) {
> log.error("Error! " + e.getMessage(), e);
> throw new UserStoreException("Error! " +
> e.getMessage(), e);
> } finally {
> DatabaseUtil.closeAllConnections(null, prepStmt); <------
> Cleaning up will happened in this method
> }
>
>
> Please send SVN pointer to the class. It is difficult to fix some
> issues like this without looking at all the code.
SVN url -
https://svn.wso2.org/repos/wso2/trunk/carbon/core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/claim/dao/ClaimDAO.java
See "protected void addClaimMapping(Connection dbConnection,
ClaimMapping claimMapping)
throws UserStoreException" method.
Note : If i replace finally block code with following, i will not get
above mentioned rule violation.
} finally {
try {
prepStmt.close()
} catch (Exception e) {
...
...
}
}
>
> As you can see in above code, connections are closed in a separate
> method. (DatabaseUtil.closeAllConnections(null, prepStmt)). But still
> findbugs complains about OBL_UNSATISFIED_OBLIGATION rule
> violation. So i
> am thinking this is because that findbugs is unable to analyze method
> calls in finally block. (Also DatabaseUtil.closeAllConnections is
> not an
> api method)
> Correct me if i am wrong.
>
> 2. RC_REF_COMPARISON - Suspicious reference comparison
> Sample code
>
> if (sr.getLastNodeAllowedAccess() == Boolean.TRUE) {
> return true;
> }
>
>
> Why is the code written that way instead of, if
> (sr.getLastNodeAllowedAccess()) {}. Such code needs to be fixed.
The returned value from sr.getLastNodeAllowedAccess() can have one
following possibilities.
1. Boolean.TRUE
2. Boolean.FALSE
3. Null
Null is used to propagate permissions across a hierarchy. i.e. if
particular node in the permission tree returns null for access, that
node's permission is decided by checking the value of its parent.
Therefore we are using wrapped Boolean class rather than primitive booleans.
>
>
> This basically complains about comparison over Boolean.TRUE static
> value. Our permission tree, code is heavily using Boolean.TRUE and
> Boolean.FALSE variables. There are many places, which use above 2
> variables. Changing those variables will affect many code paths.
> On the
> other hand user core code is well tested and using, Boolean.TRUE,
> Boolean.FALSE will not do any harm as they are static members in
> Boolean
> class. So i am yet to decide whether to change this.
> Feedback appreciated on this.
>
> 3. ODR_OPEN_DATABASE_RESOURCE - This is similar to in 1. Hope we can
> ignore this since we call DatabaseUtil.closeAllConnections method.
>
> 4. URF_UNREAD_FIELD - Unread field
>
> org.wso2.carbon.user.core.profile.DefaultProfileConfigurationManager
> has a private field called "tenantId". This is initialized in
> constructor but never read. Hope it is ok to remove this field. Also i
> found an unused non private tenantId variable in
>
> org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/claim/DefaultClaimManager.java.
> I removed that as well. (Not yet committed.)
>
>
> Please remove unused private attributes.
>
>
>
> I am in the process of doing some major changes to apacheds (with the
> introduction of KDC) component. I am fixing findbugs issues, in
> parallel
> to those changes. I will commit code once i am done.
>
>
> Cool!
>
>
> Thanks
> AmilaJ
>
>
> Afkham Azeez wrote:
> > I'm expecting a response, at least from the folks whose names have
> > been listed below. We also need more volunteers.
> >
> > Azeez
> >
> > On Fri, Sep 24, 2010 at 9:57 AM, Afkham Azeez <[email protected]
> <mailto:[email protected]>
> > <mailto:[email protected] <mailto:[email protected]>>> wrote:
> >
> > Folks,
> > I'd like to see more enthusiasm in this area. I'm sure that
> all of
> > you agree with me that this exercise will have a long term
> > benefit. Generally, tasks like this get pushed to the bottom
> since
> > the immediate benefit may not be apparent.
> >
> > I'd like to request the following volunteers to dedicate
> some time
> > *today* to finish off these items.
> >
> > 1. Isuru
> > 2. Thilina
> > 3. Senaka
> > 4. Sameera
> > 5. Ruwan
> > 6. AmilaJ
> > 7. Ajith
> > 8. Rajika
> > 9. Ratha
> > 10. Shariq
> > 11. Kasun
> > 12. Hiranya
> > 13. Ruchira
> > 14. Supun
> > 15. Heshan
> > 16. Chathuri
> >
> > Unfortunately, there are many component which do not have any
> > volunteers. At least the initial authors of these components
> > should take ownership of these components! If anybody has
> issues,
> > you could use the IRC channel to communicate your issues.
> >
> > Thanks
> > Azeez
> >
> >
> >
> >
> > On Tue, Sep 21, 2010 at 2:09 PM, Afkham Azeez
> <[email protected] <mailto:[email protected]>
> > <mailto:[email protected] <mailto:[email protected]>>> wrote:
> >
> > Hi folks,
> > We are planning to have $subject this Thursday at the #59
> > office. Volunteers please be in office at 9.30 AM.
> >
> > See:
>
> https://spreadsheets.google.com/a/wso2.com/ccc?key=0AnW7aoJmJW4wdGlnRGR0OVNRSFRmTHo0dHlMbHJMM0E&hl=en#gid=2
>
> <https://spreadsheets.google.com/a/wso2.com/ccc?key=0AnW7aoJmJW4wdGlnRGR0OVNRSFRmTHo0dHlMbHJMM0E&hl=en#gid=2>
> >
>
> <https://spreadsheets.google.com/a/wso2.com/ccc?key=0AnW7aoJmJW4wdGlnRGR0OVNRSFRmTHo0dHlMbHJMM0E&hl=en#gid=2
>
> <https://spreadsheets.google.com/a/wso2.com/ccc?key=0AnW7aoJmJW4wdGlnRGR0OVNRSFRmTHo0dHlMbHJMM0E&hl=en#gid=2>>
> >
> > Thanks
> > --
> > *Afkham Azeez*
> > Senior Software Architect & Senior Manager; WSO2, Inc.;
> > http://wso2.com,
> > /
> > /
> > /Member; Apache Software Foundation;
> //http://www.apache.org///
> > email: //[email protected]/ <http://[email protected]/>
> <mailto:[email protected] <mailto:[email protected]>>/ cell: +94 77
> > 3320919
> > blog: //http://blog.afkham.org//
> > twitter: //http://twitter.com/afkham_azeez//
> > linked-in: //http://lk.linkedin.com/in/afkhamazeez/
> > /
> > /
> > /Lean . Enterprise . Middleware/
> >
> >
> >
> >
> >
> > --
> > *Afkham Azeez*
> > Senior Software Architect & Senior Manager; WSO2, Inc.;
> > http://wso2.com,
> > /
> > /
> > /Member; Apache Software Foundation; //http://www.apache.org///
> > email: //[email protected]/ <http://[email protected]/>
> <mailto:[email protected] <mailto:[email protected]>>/ cell: +94 77 3320919
> > blog: //http://blog.afkham.org//
> > twitter: //http://twitter.com/afkham_azeez//
> > linked-in: //http://lk.linkedin.com/in/afkhamazeez/
> > /
> > /
> > /Lean . Enterprise . Middleware/
> >
> >
> >
> >
> > --
> > *Afkham Azeez*
> > Senior Software Architect & Senior Manager; WSO2, Inc.;
> http://wso2.com,
> > /
> > /
> > /Member; Apache Software Foundation; //http://www.apache.org///
> > email: //[email protected]/ <http://[email protected]/>
> <mailto:[email protected] <mailto:[email protected]>>/ cell: +94 77 3320919
> > blog: //http://blog.afkham.org//
> > twitter: //http://twitter.com/afkham_azeez//
> > linked-in: //http://lk.linkedin.com/in/afkhamazeez/
> > /
> > /
> > /Lean . Enterprise . Middleware/
> >
> >
> ------------------------------------------------------------------------
> >
> > _______________________________________________
> > Carbon-dev mailing list
> > [email protected] <mailto:[email protected]>
> > https://mail.wso2.org/cgi-bin/mailman/listinfo/carbon-dev
> >
>
>
> _______________________________________________
> Carbon-dev mailing list
> [email protected] <mailto:[email protected]>
> https://mail.wso2.org/cgi-bin/mailman/listinfo/carbon-dev
>
>
>
>
> --
> *Afkham Azeez*
> Senior Software Architect & Senior Manager; WSO2, Inc.; http://wso2.com,
> /
> /
> /Member; Apache Software Foundation; //http://www.apache.org///
> email: //[email protected]/ <mailto:[email protected]>/ cell: +94 77 3320919
> blog: //http://blog.afkham.org//
> twitter: //http://twitter.com/afkham_azeez//
> linked-in: //http://lk.linkedin.com/in/afkhamazeez/
> /
> /
> /Lean . Enterprise . Middleware/
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Carbon-dev mailing list
> [email protected]
> https://mail.wso2.org/cgi-bin/mailman/listinfo/carbon-dev
>
_______________________________________________
Carbon-dev mailing list
[email protected]
https://mail.wso2.org/cgi-bin/mailman/listinfo/carbon-dev
