Skip to content

Fix clang tidy ci #2375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 9, 2025
Merged

Fix clang tidy ci #2375

merged 1 commit into from
May 9, 2025

Conversation

justsmth
Copy link
Contributor

@justsmth justsmth commented Apr 28, 2025

Issues:

Addresses P229174291

Description of changes:

Fixes usage of clang-tidy-review:

  • Build needed to be performed inside docker image so that source files would have correct paths in the compile_commands.json
    Made changes to clang-tidy-review to preserve comments that are very close to lines in the diff
  • justsmth/clang-tidy-review@e8b24ea

Testing:

  • I added a bad commit to this PR so that clang-tidy review would make a comment. See below.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@justsmth justsmth force-pushed the fix-clang-tidy-ci branch 23 times, most recently from b8443c3 to 9b40ed0 Compare April 29, 2025 19:07
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

crypto/bio/bio.c Outdated
@@ -443,7 +443,7 @@ int BIO_flush(BIO *bio) {
}

long BIO_ctrl(BIO *bio, int cmd, long larg, void *parg) {
if (bio == NULL) {
if (bio != NULL) {
return 0;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

warning: Access to field 'method' results in a dereference of a null pointer (loaded from variable 'bio') [clang-analyzer-core.NullDereference]

  if (bio->method == NULL || bio->method->ctrl == NULL) {
      ^
Additional context

crypto/bio/bio.c:598: Passing value via 1st parameter 'bio'

  return (int)BIO_ctrl(bio, BIO_CTRL_SET_CLOSE, close_flag, NULL);
                       ^

crypto/bio/bio.c:598: Calling 'BIO_ctrl'

  return (int)BIO_ctrl(bio, BIO_CTRL_SET_CLOSE, close_flag, NULL);
              ^

crypto/bio/bio.c:445: Assuming 'bio' is equal to NULL

  if (bio != NULL) {
      ^

crypto/bio/bio.c:445: Taking false branch

  if (bio != NULL) {
  ^

crypto/bio/bio.c:449: Access to field 'method' results in a dereference of a null pointer (loaded from variable 'bio')

  if (bio->method == NULL || bio->method->ctrl == NULL) {
      ^

@justsmth justsmth force-pushed the fix-clang-tidy-ci branch 3 times, most recently from a1262f4 to 4109d21 Compare April 29, 2025 19:27
@justsmth justsmth changed the title [DRAFT] Fix clang tidy ci Fix clang tidy ci Apr 29, 2025
@justsmth justsmth marked this pull request as ready for review April 29, 2025 19:32
@justsmth justsmth requested a review from a team as a code owner April 29, 2025 19:32
@justsmth justsmth requested a review from andrewhop April 29, 2025 19:32
@codecov-commenter
Copy link

codecov-commenter commented Apr 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.80%. Comparing base (4f5b41e) to head (bab60d3).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2375      +/-   ##
==========================================
- Coverage   78.81%   78.80%   -0.02%     
==========================================
  Files         621      621              
  Lines      108417   108417              
  Branches    15399    15399              
==========================================
- Hits        85447    85435      -12     
- Misses      22299    22312      +13     
+ Partials      671      670       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@justsmth justsmth force-pushed the fix-clang-tidy-ci branch 4 times, most recently from 40589ff to 5f5ed93 Compare April 29, 2025 20:53
@@ -11,6 +11,5 @@ Checks: '
modernize-*,
performance-*,
portability-*,
readability-*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "readability-*" lints are very noisy in our code base.

@justsmth justsmth force-pushed the fix-clang-tidy-ci branch from 5f5ed93 to bab60d3 Compare May 9, 2025 12:44
@justsmth justsmth merged commit bff0eaa into aws:main May 9, 2025
112 of 114 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants