Bug 55938

Summary: clang-analyzer report for 1.1.31
Product: Tomcat Native Reporter: Ville Skyttä <ville.skytta>
Component: LibraryAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: 1.1.31   
Target Milestone: ---   
Hardware: All   
OS: All   
URL: http://scop.fedorapeople.org/scan-build/tomcat-native-1.1.29/

Description Ville Skyttä 2013-12-29 12:21:31 UTC
FYI, running clang-analyzer's scan-build on tcnative 1.1.29 flags some issues:
http://scop.fedorapeople.org/scan-build/tomcat-native-1.1.29/
Comment 1 Christopher Schultz 2013-12-30 02:57:53 UTC
Some comments about the report:

1. The 2 "null dereference" items are from functions  that AFAICT are never used (jbs_new and jbs_read). Normally the code identified by the compiler here would be considered a big problem (they are obvious logic errors) but it's not really a big deal given the fact that these functions are not used.

2. The dead assignments should probably be investigated. I didn't look at all of them, but for example, network.c:722 has an obvious logic error in that GetByteArrayElements gets called twice under certain circumstances when a single call appears to be appropriate.

3. The 'nonnull' arguments should also b e investigated.

I suspect a lot of the cruft found by the compiler, here, can be completely removed from the code.
Comment 2 Ville Skyttä 2014-02-02 19:12:00 UTC
Updated report done with clang 3.4 is at the same location.
Comment 3 Konstantin Kolinko 2014-07-02 09:08:22 UTC
Thank you.

An obvious "Dead assignment" issue in ssl.c fixed with r1607271.

Two "Dereference of null pointer" issues in ssl.c fixed with r1607278.

A "Dead assignment" issue in Socket.sendto() (network.c) fixed with r1607296.

The fixes will be in TCNative 1.1.31


Two "Dead assignment" bugs in File.writeFull(), File.write() (file.c):
- I think the fix should be on the Java side to remove "-1" as allowed value for size in Javadoc for these methods, and also in File.read() and others.
- The unused GetArrayLength() calls can be removed from C code.

Motivation:
1). The code does not work, and nobody complained.
2). It is easy to measure array length in Java. There is no need for this feature.
3). While is is easy to fix the assignment (it should assign to "nbytes" variable), the code is wrong and dangerous. It shall subtract "offset" from the array length, as "offset" can be non-zero.


"Dead assignment" in SSL_callback_SSL_verify (sslutils.c) is some TODO code. No issue.


"Argument with 'nonnull' attribute passed null" issues in Pool.dataGet(), Pool.dataSet() (pool.c) shall be taken care in Java code by never passing null as the value of "key". No issues here.
Comment 4 Ville Skyttä 2014-07-12 09:36:00 UTC
Updated report done with clang 3.4.1 found no new issues, it's at
http://scop.fedorapeople.org/scan-build/tomcat-native-1.1.31/
Comment 5 Mark Thomas 2017-01-31 15:15:54 UTC
Fixed for 1.2.11