TS-4532: static type checking for time #1

SolidWallOfCode · 2016-09-06T03:04:06Z

This is preliminary effort to get a feel for using std::chrono. Sections converted are

lib/ts
iocore/eventsystem
iocore/net - UnixNetVConnection and UnixNet plus required header files.

Rough first pass just for iocore/eventsystem to get a feel for the changes. Changed lib/ts, iocore/eventsystem to compile. Partial conversion in iocore/net.

…led from remap. (#1) Merged

…led from remap. (#1)

Problem: CID 1021924 (#1 of 1): Missing break in switch (MISSING_BREAK) unterminated_case: The case for value TS_EVENT_VCONN_WRITE_READY is not terminated by a 'break' statement. Solution: It was intended not to have a break (not a bug), so refactored the code to make coverity happy.

Problem: CID 1021925 (#1 of 1): Missing break in switch (MISSING_BREAK) unterminated_case: The case for value TS_EVENT_VCONN_WRITE_COMPLETE is not terminated by a 'break' statement. Solution: It was intended not to have a break (not a bug), so refactored the code to make coverity happy.

Problem: CID 1022060 (#1 of 1): Unchecked return value (CHECKED_RETURN) Solution: Added a check for the return value.

Problem: CID 1200018 (#1 of 1): Resource leak (RESOURCE_LEAK) 13. overwrite_var: Overwriting token in token = _TSstrdup(val, -1L, "memory/secure-link/secure-link.c:70") leaks the storage that token points to. CID 1200017 (#1 of 1): Resource leak (RESOURCE_LEAK) 19. overwrite_var: Overwriting expire in expire = _TSstrdup(val, -1L, "memory/secure-link/secure-link.c:72") leaks the storage that expire points to Fix: Took the TSstrdup() for expire and token outside the loop to avoid the leak.

Problem: CID 1373288 (#1 of 1): Dereference after null check (FORWARD_NULL) 20. var_deref_model: Passing null pointer expire to strtol, which dereferences it. Fix: Missing expiration query parameter and missing expiration query parameter value should be treated the same (expire=0).

Problem: CID 1373289 (#1 of 1): Explicit null dereferenced (FORWARD_NULL) 21. var_deref_model: Passing null pointer token to strcmp, which dereferences it. Fix: Check if token is NULL and if yes it cannot be equal to what is in hash (also hash is never NULL)

Problem: CID 1356971 (#1 of 1): Unchecked return value (CHECKED_RETURN) 13. check_return: Calling TSUrlCreate without checking return value Fix: Added the result check and also fixed a resource leak.

Problem: CID 1022124 (#1 of 1): Dereference before null check (REVERSE_INULL) check_after_deref: Null-checking txn_sm suggests that it may be null, but it has already been dereferenced on all paths leading to the check. Fix: Removed the check for NULL.

Problem: CID 1022122 (#1 of 1): Dereference before null check (REVERSE_INULL) check_after_deref: Null-checking txn_sm->q_client_request_buffer suggests that it may be null, but it has already been dereferenced on all paths leading to the check. CID 1022123 (#1 of 1): Dereference before null check (REVERSE_INULL) check_after_deref: Null-checking txn_sm->q_client_response_buffer suggests that it may be null, but it has already been dereferenced on all paths leading to the check. CID 1022125 (#1 of 1): Dereference before null check (REVERSE_INULL) check_after_deref: Null-checking txn_sm->q_cache_read_buffer suggests that it may be null, but it has already been dereferenced on all paths leading to the check Fix: Checked the return values on each step instead of all at the end.

Problem: CID 1022126 (#1 of 1): Dereference before null check (REVERSE_INULL) check_after_deref: Null-checking txn_sm->q_server_response_buffer suggests that it may be null, but it has already been dereferenced on all paths leading to the check. CID 1022127 (#1 of 1): Dereference before null check (REVERSE_INULL) check_after_deref: Null-checking txn_sm->q_server_request_buffer suggests that it may be null, but it has already been dereferenced on all paths leading to the check. Fix: Checked the return values on each step instead of all at the end.

Problem: CID 1363659 (#1 of 1): Dereference before null check (REVERSE_INULL) check_after_deref: Null-checking sp->server.vconn suggests that it may be null, but it has already been dereferenced on all paths leading to the check. Fix: Removed the check the TSReleaseAssert(sp->server.vconn != nullptr); should make sure we don't dereference nullptr.

Problem: CID 1373301 (#1 of 1): Untrusted loop bound (TAINTED_SCALAR) 17. tainted_data: Using tainted variable msg_len as a loop boundary. Fix: Added check for p < ws_buf_.size() to make sure improper msg_len (user input) would not lead to out of scope write.

Issue: CID 1375841 (#1 of 1): Buffer not null terminated (BUFFER_SIZE) 1. buffer_size: Calling strncpy with a source string whose length (4 chars) is greater than or equal to the size argument (4) will fail to null-terminate key. Fix: The code is correct. The buffer is not meant to be NULL-terminated. It seems Coverity thinks that since strncpy is used NULL-terminated buffer is expected. Changing strncpy to memcpy. Also removing unnecessary #undef

Problem: CID 1200018 (#1 of 1): Resource leak (RESOURCE_LEAK) 13. overwrite_var: Overwriting token in token = _TSstrdup(val, -1L, "memory/secure-link/secure-link.c:70") leaks the storage that token points to. CID 1200017 (#1 of 1): Resource leak (RESOURCE_LEAK) 19. overwrite_var: Overwriting expire in expire = _TSstrdup(val, -1L, "memory/secure-link/secure-link.c:72") leaks the storage that expire points to Fix: Took the TSstrdup() for expire and token outside the loop to avoid the leak. (cherry picked from commit da94824)

Issue: CID 1375841 (#1 of 1): Buffer not null terminated (BUFFER_SIZE) 1. buffer_size: Calling strncpy with a source string whose length (4 chars) is greater than or equal to the size argument (4) will fail to null-terminate key. Fix: The code is correct. The buffer is not meant to be NULL-terminated. It seems Coverity thinks that since strncpy is used NULL-terminated buffer is expected. Changing strncpy to memcpy. Also removing unnecessary #undef (cherry picked from commit 9a4aa33)

github-actions · 2021-06-21T18:32:47Z

This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.

Fix the `make check` failure with the following output by changing to .gold which meets our license exclusion regex rules: Printing headers for text files without a valid license header... ===================================================== == File: ./tests/gold_tests/records/gold/renamed_records.out ===================================================== ``` ┌■ 8 Renamed records: └┬──» #1 : proxy.config.output.logfile -> proxy.config.output.logfile.name ├──» #2 : proxy.config.exec_thread.autoconfig -> proxy.config.exec_thread.autoconfig.enabled ├──» #3 : proxy.config.hostdb -> proxy.config.hostdb.enabled ├──» #4 : proxy.config.tunnel.prewarm -> proxy.config.tunnel.prewarm.enabled ├──» #5 : proxy.config.ssl.origin_session_cache -> proxy.config.ssl.origin_session_cache.enabled ├──» #6 : proxy.config.ssl.session_cache -> proxy.config.ssl.session_cache.value ├──» #7 : proxy.config.ssl.TLSv1_3 -> proxy.config.ssl.TLSv1_3.enabled └──» #8 : proxy.config.ssl.client.TLSv1_3 -> proxy.config.ssl.client.TLSv1_3.enabled ```

CID 1508856: Logically dead code apache#10432 In access_control.cc: ```cpp 197 size_t equalsign = kvp.find(_tokenConfig.kvDelimiter); cond_cannot_single: Condition 18446744073709551615UL == equalsign, taking false branch. Now the value of equalsign cannot be equal to -1. 198 if (kvp.npos == equalsign) { 199 ERROR_OUT("invalid key-value-pair, missing key-value delimiter"); 200 return _state = INVALID_SYNTAX; 201 } 202 StringView key = kvp.substr(0, equalsign); cannot_single: At condition equalsign != 18446744073709551615UL, the value of equalsign cannot be equal to -1. dead_error_condition: The condition equalsign != 18446744073709551615UL must be true. CID 1508856 (#1 of 1): Logically dead code (DEADCODE) dead_error_line: Execution cannot reach this statement: <temporary>.basic_string_vi.... 203 StringView value = equalsign != kvp.npos ? kvp.substr(equalsign + 1) : ""; ```

SolidWallOfCode added 3 commits September 5, 2016 21:40

TS-4532: Static type checking for time.

f06f2cb

Rough first pass just for iocore/eventsystem to get a feel for the changes. Changed lib/ts, iocore/eventsystem to compile. Partial conversion in iocore/net.

TS-4532; Trying out using ink_zero overload to set durations to zero.

8592ab7

TS-4532: More tweaks.

3a7d43e

SolidWallOfCode added a commit that referenced this pull request Nov 2, 2016

TS-4593: Extend IPAllow to filter outbound connections and be control…

b92a6b3

…led from remap. (#1) Merged

SolidWallOfCode added a commit that referenced this pull request Feb 1, 2017

TS-4593: Extend IPAllow to filter outbound connections and be control…

e541986

…led from remap. (#1)

SolidWallOfCode pushed a commit that referenced this pull request May 9, 2017

Coverity 1022060: Unchecked return value

cf27627

Problem: CID 1022060 (#1 of 1): Unchecked return value (CHECKED_RETURN) Solution: Added a check for the return value.

github-actions bot added the Stale label Jun 21, 2021

github-actions bot closed this Jun 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

TS-4532: static type checking for time #1

TS-4532: static type checking for time #1

Uh oh!

SolidWallOfCode commented Sep 6, 2016

Uh oh!

github-actions bot commented Jun 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

TS-4532: static type checking for time #1

TS-4532: static type checking for time #1

Uh oh!

Conversation

SolidWallOfCode commented Sep 6, 2016

Uh oh!

github-actions bot commented Jun 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants