New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix huntbugs warnings #586

Open
vlsi opened this Issue Jun 20, 2016 · 0 comments

Comments

Projects
None yet
1 participant
@vlsi
Member

vlsi commented Jun 20, 2016

Here's the way to get huntbugs report:

cd pgjdbc
mvn one.util:huntbugs-maven-plugin:0.0.9:huntbugs

The report is generated into target/huntbugs/report.html
All the warnings (88 in total) look reasonable.

Note: it would be nice if you could build huntbugs from source (it is in active development, and new checkers are added quite frequently)

It would be nice to fix those (e.g. PR per warning type).

For instance

All warnings (88)

                                                  All warnings (88)
Invocation of toString() on an array                            Invocation of toString() on an array in
(ArrayToString [x])                                             TimestampUtils static {}

Category: Correctness                                           The code invokes toString() on an array or implicitly
 Score:   60                                                    converts an array into String. This will generate a
Location: TimestampUtils.java:76                                fairly useless result such as [C@16f0472. Consider
 Class:   org/postgresql/jdbc/TimestampUtils                    using java.util.Arrays.toString to convert the array
 Method:  <clinit>                                              into a readable String that gives the contents of the
                                                                array.

Useless bitwise-and operation with -1
(UselessAndWithMinusOne [x])                                    Useless bitwise-and with -1 (0xffffffff) in
                                                                UnixCrypt.des_set_key().
Category: RedundantCode
 Score:   60
Location: UnixCrypt.java:444                                    This code performs bit operation like (x &
 Class:   org/postgresql/util/UnixCrypt                         0xffffffff). Such operation is useless as the result
 Method:  des_set_key                                           is always x.

Useless bitwise-and operation with -1
(UselessAndWithMinusOne [x])                                    Useless bitwise-and with -1 (0xffffffff) in
                                                                UnixCrypt.des_set_key().
Category: RedundantCode
 Score:   60
Location: UnixCrypt.java:448
 Class:   org/postgresql/util/UnixCrypt                         This code performs bit operation like (x &
 Method:  des_set_key                                           0xffffffff). Such operation is useless as the result
Variable: s                                                     is always x.


Static field should be final
(StaticFieldShouldBeFinal [x])                                  Static field
                                                                SSPIClient.SSPI_DEFAULT_SPN_SERVICE_CLASS should be
Category: MaliciousCode                                         final.
 Score:   55
Location: SSPIClient.java:40
 Class:   org/postgresql/sspi/SSPIClient                        The field SSPIClient.SSPI_DEFAULT_SPN_SERVICE_CLASS
 Method:  <clinit>                                              is not final, but could be easily declared final to
 Field:   SSPI_DEFAULT_SPN_SERVICE_CLASS                        prevent possible malicious changes from other
                                                                packages.

Inefficient use of keySet() iterator instead of values()
iterator                                                        Inefficient use of keySet() iterator instead of
(WrongMapIteratorValues [x])                                    values() iterator in PgResultSet.insertRow().

Category: Performance
 Score:   55                                                    This method accesses the Map values, using a key that
Location: PgResultSet.java:972                                  was retrieved from a keySet() iterator. It's detected
 Class:   org/postgresql/jdbc/PgResultSet                       that map key is not used for any purpose other than
 Method:  insertRow                                             getting map value. It would be shorter and more
                                                                efficient to iterate over Map.values() instead.

                                                                Naked Object.notifyAll() call in PgStatement.cancel
Naked call to notify() or notifyAll()                           ().
(NotifyNaked [x])                                               A call to Object.notifyAll() was made without any
                                                                (apparent) accompanying modification to mutable
Category: Multithreading                                        object state. In general, calling a notify method on
 Score:   50                                                    a monitor is done because some condition another
Location: PgStatement.java:1047                                 thread is waiting for has become true. However, for
 Class:   org/postgresql/jdbc/PgStatement                       the condition to be meaningful, it must involve a
 Method:  cancel                                                heap object that is visible to both threads.
 Field:   connection                                            This bug does not necessarily indicate an error,
                                                                since the change to mutable object state may have
                                                                taken place in a method which then called the method
                                                                containing the notification.
String concatenation in a loop
(StringConcatInLoop [x])                                        Variable sql is concatenated in loop.

Category: Performance
 Score:   50
Location: PgDatabaseMetaData.java:3065                          This code concatenates a string (stored in variable
 Class:   org/postgresql/jdbc/PgDatabaseMetaData                sql) in a loop. This could be really inefficient.
 Method:  getUDTs                                               Consider using StringBuilder instead.
Variable: sql

Unconditional wait
(WaitUnconditional [x])                                         Unconditional Object.wait() call in
                                                                PgStatement.killTimerTask().
Category: Multithreading
 Score:   50
Location: PgStatement.java:1160                                 This method contains a call to Object.wait() which is
 Class:   org/postgresql/jdbc/PgStatement                       not guarded by conditional control flow. The code
 Method:  killTimerTask                                         should verify that condition it intends to wait for
 Field:   connection                                            is not already satisfied before calling wait; any
                                                                previous notifications will be ignored.

Private or package-private field is never read                  Private or package-private field
(UnreadPrivateField [x])                                        PgStatement.cancelTimerTask is is written, but never
                                                                read.
Category: RedundantCode
 Score:   48                                                    The field PgStatement.cancelTimerTask is written, but
Location: PgStatement.java:69                                   never read, so probably it should be removed. If it's
 Class:   org/postgresql/jdbc/PgStatement                       intended to be read via reflection, consider
 Method:  <init>                                                annotating it with some annotation to avoid
 Field:   cancelTimerTask                                       confusion. False-positive is possible if the field is
                                                                used only to keep strong reference to the object.

Private or package-private field is never read                  Private or package-private field
(UnreadPrivateField [x])                                        PgStatement.statementState is is written, but never
                                                                read.
Category: RedundantCode
 Score:   48                                                    The field PgStatement.statementState is written, but
Location: PgStatement.java:85                                   never read, so probably it should be removed. If it's
 Class:   org/postgresql/jdbc/PgStatement                       intended to be read via reflection, consider
 Method:  <init>                                                annotating it with some annotation to avoid
 Field:   statementState                                        confusion. False-positive is possible if the field is
                                                                used only to keep strong reference to the object.

etc etc.

@vlsi vlsi added the easy label Jun 20, 2016

AlexElin added a commit to AlexElin/pgjdbc that referenced this issue Aug 3, 2016

vlsi added a commit that referenced this issue Aug 3, 2016

AlexElin added a commit to AlexElin/pgjdbc that referenced this issue Aug 4, 2016

refactor: fix some hintbugs warnings
fix this warnings:
Invocation of toString() on an array
Inefficient use of keySet() iterator instead of values() iterator
Inefficient use of keySet() iterator instead of entrySet() iterator

relates to Fix huntbugs warnings pgjdbc#586

AlexElin added a commit to AlexElin/pgjdbc that referenced this issue Aug 4, 2016

refactor: fix some hintbugs warnings
Arrays.toString(...) -> new String(..)

relates to Fix huntbugs warnings pgjdbc#586

AlexElin added a commit to AlexElin/pgjdbc that referenced this issue Aug 4, 2016

refactor: fix some hintbugs warnings
checkstyle

relates to Fix huntbugs warnings pgjdbc#586

vlsi added a commit that referenced this issue Aug 4, 2016

refactor: fix some hintbugs warnings (#616)
Invocation of toString() on an array
Inefficient use of keySet() iterator instead of values() iterator
Inefficient use of keySet() iterator instead of entrySet() iterator

relates to Fix huntbugs warnings #586

zemian pushed a commit to zemian/pgjdbc that referenced this issue Oct 6, 2016

zemian pushed a commit to zemian/pgjdbc that referenced this issue Oct 6, 2016

refactor: fix some hintbugs warnings (pgjdbc#616)
Invocation of toString() on an array
Inefficient use of keySet() iterator instead of values() iterator
Inefficient use of keySet() iterator instead of entrySet() iterator

relates to Fix huntbugs warnings pgjdbc#586

AlexElin added a commit to AlexElin/pgjdbc that referenced this issue Nov 6, 2016

refactor: use varargs
use varargs instead of new Object[]{..}
delete "final" from "private static" methods (fix warnings from pgjdbc#586)

vlsi added a commit that referenced this issue Nov 12, 2016

refactor: use varargs
use varargs instead of new Object[]{..}
delete "final" from "private static" methods (fix warnings from #586)

fixes #681
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment