Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Identifying code quality issues e.g. unnecessary null pointer checks #2616

Open
elfring opened this issue Feb 9, 2023 · 8 comments
Open

Identifying code quality issues e.g. unnecessary null pointer checks #2616

elfring opened this issue Feb 9, 2023 · 8 comments

Comments

@elfring
Copy link

elfring commented Feb 9, 2023

An extra null pointer check is not needed in functions like the following.
@slaff
Copy link
Contributor

slaff commented Feb 10, 2023

@elfring can you prepare a PR with the suggested changes?

@mikee47
Copy link
Contributor

mikee47 commented May 23, 2023

Some of those should be using std::unique_ptr as well :-)

@slaff
Copy link
Contributor

slaff commented May 23, 2023 •
edited

Some of those should be using std::unique_ptr as well :-)

@mikee47 do you want to create a PR with the necessary changes?

@elfring
Copy link
Author

elfring commented May 23, 2023

💭 Would you become interested to use a development tool like “clang-tidy” for corresponding source code adjustments?

@mikee47 mikee47 mentioned this issue May 26, 2023
Merged
@slaff
Copy link
Contributor

slaff commented May 26, 2023

Fixed in #2643

@slaff slaff closed this as completed May 26, 2023
@elfring
Copy link
Author

elfring commented May 27, 2023

🤔 I got the impression that other source code adjustments would be more appropriate for some implementation details.

@mikee47 mikee47 reopened this May 28, 2023
@mikee47
Copy link
Contributor

mikee47 commented May 28, 2023

I'm reopening this issue as it is not yet fully resolved. clang-tidy is undoubtedly useful, especially if it can be integrated into the build system in some way. Whether automated code changes are appropriate is another matter....

Are there any other offline code checking tools we might consider?

@mikee47 mikee47 changed the title Remove unnecessary null pointer checks Identifying code quality issues e.g. unnecessary null pointer checks May 28, 2023
@elfring
Copy link
Author

elfring commented May 28, 2023

Another update candidate was left over.

@mikee47 mikee47 mentioned this issue May 28, 2023
Merged
slaff pushed a commit that referenced this issue Apr 23, 2024
@mikee47
Add clang-tidy support for static code analysis (#2648)
75a7481 
This PR allows static code analysis using clang-tidy, suggested by #2616.

Clang has more limited `constexpr` support than GCC so cannot parse some of the FlashString and NanoTime code without modification. However, these changes are only made when `__clang__` is defined and do not affect regular builds with GCC.

This PR also includes some basic code fixes which clang identifies. There are a lot more to look at.

To try it out, build a project with:

```
make CLANG_TIDY=clang-tidy
```

Add additional parameters like this:

```
make CLANG_TIDY="clang-tidy --fix --fix-errors"
```

Notes:

- Don't use `-j` option as clang-tidy output and fixes don't get serialised correctly.
- Settings are in the main `.clang-tidy` file. A custom version can be used and passed on the command line.
- I've been using clang 17.0.6 https://releases.llvm.org/17.0.1/tools/clang/tools/extra/docs/clang-tidy/index.html
- Only source files which haven't been built are inspected. So, to restrict which code gets processed built the entire application then 'clean' the relevant modules before proceeding with clang-tidy.
- No object code is generated by clang.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@slaff @elfring @mikee47