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

Use standard C++ timer for long timeouts (#1327), remove unused code #1328

Merged
merged 9 commits into from
Nov 14, 2023
Merged

Use standard C++ timer for long timeouts (#1327), remove unused code #1328

merged 9 commits into from
Nov 14, 2023

Conversation

uweseimet
Copy link
Contributor

No description provided.

@uweseimet uweseimet linked an issue Nov 9, 2023 that may be closed by this pull request
Use standard C++ timer for long timeouts #1327
Closed
@uweseimet uweseimet requested a review from dialtr November 9, 2023 17:48
@uweseimet uweseimet changed the title Use standard C++ timer for long timeouts (#1327) Use standard C++ timer for long timeouts (#1327), remove unused code Nov 11, 2023
@uweseimet uweseimet marked this pull request as ready for review November 11, 2023 15:44
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Nov 12, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

rdmark
rdmark approved these changes Nov 14, 2023
View reviewed changes
Copy link
Member

@rdmark rdmark left a comment

Choose a reason for hiding this comment

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

Was it Sonar that made you aware of the bitrotted code?

cpp/hal/bus.h
@@ -68,8 +68,7 @@ class BUS : public PinControl
// Operation modes definition
enum class mode_e {
TARGET = 0,
INITIATOR = 1,
MONITOR = 2,
Copy link
Member

Choose a reason for hiding this comment

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

This is not used by scsimon?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it is not. Otherwise it would already break when compiling. You know how much I like to repeat that there is a lot of unused code ;-).

@uweseimet
Copy link
Contributor Author

uweseimet commented Nov 14, 2023 •
edited
Loading

Was it Sonar that made you aware of the bitrotted code?

No, I stumble upon this when reviewing and cleaning up the existing code. Only recently I also started with that for the code in hal. Often, as soon as you remove an unused method, for instance, another unused method or field pops up, which before was only used by the unused code you have just removed. And so on ...
And code I do not immediately understand is also quite often not needed. This is not a joke.

@uweseimet uweseimet merged commit baf89dc into develop Nov 14, 2023
15 of 16 checks passed
@uweseimet uweseimet deleted the issue_1327 branch November 14, 2023 15:03
@rdmark
Copy link
Member

rdmark commented Nov 14, 2023

Was it Sonar that made you aware of the bitrotted code?

No, I stumble upon this when reviewing and cleaning up the existing code. Only recently I also started with that for the code in hal. Often, as soon as you remove an unused method, for instance, another unused method or field pops up, which before was only used by the unused code you have just removed. And so on ...

And code I do not immediately understand is also quite often not needed. This is not a joke.

Have you tried using cppcheck for finding dead code through static analysis? I've seen it being effective in another project with a very old code base.

@uweseimet
Copy link
Contributor Author

Was it Sonar that made you aware of the bitrotted code?

No, I stumble upon this when reviewing and cleaning up the existing code. Only recently I also started with that for the code in hal. Often, as soon as you remove an unused method, for instance, another unused method or field pops up, which before was only used by the unused code you have just removed. And so on ...
And code I do not immediately understand is also quite often not needed. This is not a joke.

Have you tried using cppcheck for finding dead code through static analysis? I've seen it being effective in another project with a very old code base.

I have never heard of cppcheck. Sounds like it is something I should try.

uweseimet added a commit that referenced this pull request Nov 14, 2023
@uweseimet
Use standard C++ timer for long timeouts (#1327), remove unused code (#…
def2745 
…1328)

* Replace timer in WaitSignal()

' Remove unused code

* Remove unused file
uweseimet added a commit that referenced this pull request Nov 14, 2023
@uweseimet
Use standard C++ timer for long timeouts (#1327), remove unused code (#…
8470c1f 
…1328)

* Replace timer in WaitSignal()

' Remove unused code

* Remove unused file
uweseimet added a commit that referenced this pull request Nov 15, 2023
@uweseimet
Use standard C++ timer for long timeouts (#1327), remove unused code (#…
c14a41f 
…1328)

* Replace timer in WaitSignal()

' Remove unused code

* Remove unused file
@uweseimet
Copy link
Contributor Author

uweseimet commented Nov 17, 2023 •
edited
Loading

Was it Sonar that made you aware of the bitrotted code?

No, I stumble upon this when reviewing and cleaning up the existing code. Only recently I also started with that for the code in hal. Often, as soon as you remove an unused method, for instance, another unused method or field pops up, which before was only used by the unused code you have just removed. And so on ...
And code I do not immediately understand is also quite often not needed. This is not a joke.

Have you tried using cppcheck for finding dead code through static analysis? I've seen it being effective in another project with a very old code base.

Just tried gcccheck. It takes less than 4 seconds on my PC. And indeed it found unused code and redundant assignments and a redundant condition. Not that many code locations, but SonarCloud did not find them.
Not surprisingly almost everything reported refered to cfilesystem.c, where SonarCloud also reports about 100 issues.

rdmark pushed a commit that referenced this pull request May 1, 2024
@uweseimet @rdmark
Use standard C++ timer for long timeouts (#1327), remove unused code (#…
31be1b3 
…1328)

* Replace timer in WaitSignal()

' Remove unused code

* Remove unused file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rdmark rdmark rdmark approved these changes

@dialtr dialtr Awaiting requested review from dialtr

@akuker akuker Awaiting requested review from akuker akuker is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use standard C++ timer for long timeouts
2 participants
@uweseimet @rdmark