search

lamyj / odil

Odil is a C++11 library for the DICOM standard
Other
85 stars 21 forks source link

clang-tidy clazy warnings #82

Closed ferdymercury closed 2 years ago

ferdymercury commented 3 years ago

Some warnings are reported by clang:

/opt/odil/src/odil/Reader.cpp:137:13: warning: Value stored to 'done' is never read [clang-analyzer-deadcode.DeadStores]
 1: Value stored to 'done' is never read in /opt/odil/src/odil/Reader.cpp:137
/opt/odil/src/odil/dul/StateMachine.cpp:143:5: warning: Using assign operator but class boost::posix_time::time_duration has copy-ctor but no assign operator [clazy-rule-of-two-soft]
/opt/odil/src/odil/dul/Transport.cpp:82:5: warning: Using assign operator but class boost::posix_time::time_duration has copy-ctor but no assign operator [clazy-rule-of-two-soft]
/opt/odil/src/odil/webservices/QIDORSRequest.cpp:322:21: warning: Value stored to 'include_all' is never read [clang-analyzer-deadcode.DeadStores]
 1: Value stored to 'include_all' is never read in /opt/odil/src/odil/webservices/QIDORSRequest.cpp:322
/opt/odil/src/odil/write_ds.cpp:89:17: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
 1: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 in /opt/odil/src/odil/write_ds.cpp:89
/opt/odil/src/odil/write_ds.cpp:92:17: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
 1: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 in /opt/odil/src/odil/write_ds.cpp:92
/opt/odil/src/odil/write_ds.cpp:95:17: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
 1: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 in /opt/odil/src/odil/write_ds.cpp:95
/opt/odil/src/odil/write_ds.cpp:98:17: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
 1: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 in /opt/odil/src/odil/write_ds.cpp:98
/opt/odil/src/odil/write_ds.cpp:101:17: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
 1: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 in /opt/odil/src/odil/write_ds.cpp:101
/opt/odil/src/odil/write_ds.cpp:157:13: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
 1: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 in /opt/odil/src/odil/write_ds.cpp:157
/opt/odil/src/odil/write_ds.cpp:165:9: warning: Call to function 'strcat' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcat'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
 1: Call to function 'strcat' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcat'. CWE-119 in /opt/odil/src/odil/write_ds.cpp:165
/opt/odil/src/odil/write_ds.cpp:170:9: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
 1: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 in /opt/odil/src/odil/write_ds.cpp:170
/opt/odil/src/odil/write_ds.cpp:177:13: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
 1: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 in /opt/odil/src/odil/write_ds.cpp:177
/opt/odil/src/odil/write_ds.cpp:191:13: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
 1: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 in /opt/odil/src/odil/write_ds.cpp:191
DimitriPapadopoulos commented 2 years ago

About this one: warning: Value stored to 'include_all' is never read

It's just the result of a TODO, it will fix by itself at some point: https://github.com/lamyj/odil/blob/dc8d063d8ecf1fa787a3e2de7f5b9a9574fa1105/src/odil/webservices/QIDORSRequest.cpp#L372-L377

DimitriPapadopoulos commented 2 years ago

These warnings originate in Boost as far as I can see:

src/odil/dul/StateMachine.cpp:143:5: warning: Using assign operator but class boost::posix_time::time_duration has copy-ctor but no assign operator [clazy-rule-of-two-soft]
src/odil/dul/Transport.cpp:82:5: warning: Using assign operator but class boost::posix_time::time_duration has copy-ctor but no assign operator [clazy-rule-of-two-soft]

The source is here: https://github.com/boostorg/date_time/blob/develop/include/boost/date_time/posix_time/posix_time_duration.hpp

DimitriPapadopoulos commented 2 years ago

The strcpy/strcat warnings do make sense, but fixing them may require changing some functions, such as roundat(), by passing the buffer length in addition to the buffer itself: https://github.com/lamyj/odil/blob/dc8d063d8ecf1fa787a3e2de7f5b9a9574fa1105/src/odil/write_ds.cpp#L81-L90

That said, at first glance, the calling code does seem to be checking buffer sizes.

By the way, there's a bug in clang itself, it should also warn to replace sprintf() with snprintf(): https://github.com/lamyj/odil/blob/dc8d063d8ecf1fa787a3e2de7f5b9a9574fa1105/src/odil/write_ds.cpp#L100-L105

lamyj commented 2 years ago

Thank you @ferdymercury and @DimitriPapadopoulos for the report and proposed fixes