Skip to content

Conversation

@soumyajit11
Copy link
Contributor

CID 1508856: Logically dead code #10432

In access_control.cc:

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) : "";

Copy link
Contributor

@JosiahWI JosiahWI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing the code! I checked and this fix is also correct when '=' is the very last character in the view.
Edit: this code has good unit test coverage as well.

@bneradt
Copy link
Contributor

bneradt commented Oct 15, 2023

@soumyajit11 : @JosiahWI pointed out that your osx build should work once you rebase on top of latest master to pull in a fix for including openssl. Can you please rebase and push again? I'm guessing the autest failure is spurious and will pass when you re-run CI via your push.

@bryancall bryancall added this to the 10.0.0 milestone Oct 16, 2023
@bryancall bryancall requested a review from bneradt October 16, 2023 22:11
@apache apache deleted a comment from ezelkow1 Oct 16, 2023
@bneradt bneradt merged commit 0be42cb into apache:master Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants