Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Fix codeql t3c false positives#5923

Merged
mitchell852 merged 1 commit intoapache:masterfrom
rob05c:fix-codeql-t3c-false-positives
Jun 8, 2021
Merged

Fix codeql t3c false positives#5923
mitchell852 merged 1 commit intoapache:masterfrom
rob05c:fix-codeql-t3c-false-positives

Conversation

@rob05c
Copy link
Copy Markdown
Member

@rob05c rob05c commented Jun 8, 2021

"Fixes" codeql false positives.

None of these are actual issues. Checking for overflows is deceptive and not useful. For example, the Parent Rank being 2147483649 is no more invalid than 2147483645, but we don't have a hard limit on Parent Rank, so there's no sane max we can impose.

But this is the path of least resistance. Much as it frustrates me to make code worse for bad tools, this will make tools stop bothering us every few months to fix things they don't have the context to understand aren't issues.

No new tests, code already has tests, and any specific tests around the overflow would be deceptive, misleading, and fallacious.
No docs, no interface change.
No changelog, no interface change, and these aren't real bugs.

  • This PR is not related to any other Issue

Which Traffic Control components are affected by this PR?

  • Traffic Ops ORT

What is the best way to verify this PR?

Run tests. Observe code is obviously identical in behavior, except for overflows astronomically larger than valid values.

If this is a bug fix, what versions of Traffic Control are affected?

Not a bug fix.

The following criteria are ALL met by this PR

  • This PR includes tests OR I have explained why tests are unnecessary
  • This PR includes documentation OR I have explained why documentation is unnecessary
  • This PR includes an update to CHANGELOG.md OR such an update is not necessary
  • This PR includes any and all required license headers
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

Additional Information

@rob05c rob05c requested a review from mitchell852 June 8, 2021 18:01
@mitchell852 mitchell852 added the cache-config Cache config generation label Jun 8, 2021
@mitchell852 mitchell852 merged commit 3f2691f into apache:master Jun 8, 2021
@ocket8888
Copy link
Copy Markdown
Contributor

Alternatively, CodeQL scanning messages can just be dismissed as false positives by any committer

@rob05c
Copy link
Copy Markdown
Member Author

rob05c commented Jun 8, 2021

I almost did that. But I figured it'd just pop up again with some other "vet" tool or person in a few months

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cache-config Cache config generation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants