Skip to content

Improved addon execution errorhandling - #5451

Merged
danmar merged 15 commits into
cppcheck-opensource:mainfrom
firewave:addon-xx
Sep 20, 2023
Merged

Improved addon execution errorhandling#5451
danmar merged 15 commits into
cppcheck-opensource:mainfrom
firewave:addon-xx

Conversation

@firewave

@firewavefirewave commented Sep 14, 2023

Copy link
Copy Markdown
Collaborator

No description provided.

@firewave

firewave commented Sep 14, 2023

Copy link
Copy Markdown
CollaboratorAuthor

Preparations for #4958.

@firewave
firewaveforce-pushed the addon-xx branch 5 times, most recently from d813d87 to 85836bdCompareSeptember 15, 2023 21:00
@firewavefirewave changed the title Improved addon error handlingImproved addon execution errorhandlingSep 15, 2023
@firewave
firewaveforce-pushed the addon-xx branch 2 times, most recently from 523efd6 to 036b0e9CompareSeptember 15, 2023 21:31
@firewave
firewave marked this pull request as ready for review September 15, 2023 21:54
danmar
danmar reviewed Sep 18, 2023
View reviewed changes
Comment threadcli/cppcheckexecutor.h Outdated

/**
* Execute a shell command and read the output from it. Returns true if command terminated successfully.
* Execute a shell command and read the output from it. Returns exitcode of the command terminated successfully.

@danmardanmarSep 18, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is this sentence by intention?

Returns exitcode of the command terminated successfully.

Well it needs to be clarified somehow because the exitcode can be nonzero and as I understand it it's always a success exitcode.

@firewavefirewaveSep 18, 2023

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Just a typo.

firewave
firewave commented Sep 18, 2023
View reviewed changes
Comment threadaddons/README.md
Checks Linux system for [year 2038 problem](https://en.wikipedia.org/wiki/Year_2038_problem) safety. This required [modified environment](https://github.com/3adev/y2038). See complete description [here](https://github.com/danmar/cppcheck/blob/main/addons/doc/y2038.txt).
+ [threadsafety.py](https://github.com/danmar/cppcheck/blob/main/addons/threadsafety.py)
Analyse Cppcheck dump files to locate threadsafety issues like static local objects used by multiple threads.
+ [naming.py](https://github.com/danmar/cppcheck/blob/main/addons/naming.py)

@firewavefirewaveSep 18, 2023

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

I saw that I forgot to finish the documentation.

firewave
firewave commented Sep 18, 2023
View reviewed changes
Comment threadaddons/README.md
@firewave

firewave commented Sep 18, 2023

Copy link
Copy Markdown
CollaboratorAuthor

I updated the documentation a bit but we need to document all addons properly. I added a ticket about that: https://trac.cppcheck.net/ticket/12001.

@firewave
CppCheck: split up checks in executeAddon() / test-other.py: added …
bc30cc1
…some addon-related tests
@firewave
CppCheck: removed duplicated check
8b6ee96
@firewave
CppCheck: use alias for executeCommand type
5c3f0b0
@firewave
return exitcode code from command execution
a7a9e36
@firewave
CppCheck: parse JSON without std::istringstream
e6f46e4
@firewave
CppCheck: parse addon result only once
e8a6255
@firewave
InternalError: report INTERNAL as internalError
950ea2d
@firewave
InternalError: extracted Type to string mapping to separate function
c17784e
@firewave
InternalError: added details field and make it possible to specify …
a200b91
…additional message
@firewave
CppCheck: extracted Python detection into separate function
b1f9fc7
@firewave
CppCheck: perform Python detection only once
e08c880
@firewave
CppCheck: pass additional verbose infos about execution failure as `I…
655fd22
…nternalError::details`
@firewave
corrected exitcode of command execution
2aaef14
@firewave
avoid cut off string in verbose command execution failure
b17af05
@firewave
AddonInfo: fixed name on Windows
2beca16
@firewave

firewave commented Sep 18, 2023

Copy link
Copy Markdown
CollaboratorAuthor

We should clean up the addon folder so it only contains the actual addons.

misra_9.py looked like a separate addon and it also runs with issue but it is not. Probably need to improve the errorhandling a bit more. But that is outside of the scope of this PR.

The tests should be in the main test folder. Internal implementation files should be in a subfolder. So should be the example configurations.

We might also drop some addons which might not provide much value or reat them as examples and thus also moving them out of the main addons folder.

We also need to dogfood this stuff if we ship this as a an official addon (treating it as example might allow us to treat it as "untested and unmaintained" as well).

danmar
danmar reviewed Sep 19, 2023
View reviewed changes

@danmardanmar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

looks good as far as I can tell

Comment threadaddons/README.md
Comment threadlib/cppcheck.cpp
@danmar
danmar merged commit a43b55a into cppcheck-opensource:mainSep 20, 2023
@firewave
firewave deleted the addon-xx branch September 20, 2023 09:26
@firewavefirewave mentioned this pull request Sep 25, 2023
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@danmardanmardanmar left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@firewave@danmar