Skip to content

Conversation

@olabetskyi
Copy link
Collaborator

No description provided.

@olabetskyi
Copy link
Collaborator Author

We are not checking yet if it's valid code. Just a first take

@danmar
Copy link
Owner

danmar commented Feb 9, 2024

I think a TestSymbolDatabase test case would be useful to show that it works as intended. And then it would be interesting if you could run the test-my-pr.py script for a while and see what happens..

@olabetskyi
Copy link
Collaborator Author

my_check_diff.log
256 packets were checked no diff

@olabetskyi olabetskyi marked this pull request as ready for review February 9, 2024 15:25
@chrchr-github
Copy link
Collaborator

extern is not required to declare a function, see https://trac.cppcheck.net/ticket/11555
Do we handle e.g. extern int (i);?

@olabetskyi
Copy link
Collaborator Author

extern is not required to declare a function, see https://trac.cppcheck.net/ticket/11555 Do we handle e.g. extern int (i);?

IsFunction checks for keywords. But overall the solution is for this ticket 12420 which is based around extern functions.

Copy link
Owner

@danmar danmar left a comment

Choose a reason for hiding this comment

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

nit

@danmar
Copy link
Owner

danmar commented Feb 12, 2024

Do we handle e.g. extern int (i);?

Yes.. I tested this code:

void foo() {
    extern int x();  // <- cppcheck sets function pointer on "x"
    extern int (i);   // <- cppcheck does not set function pointer
}

@danmar
Copy link
Owner

danmar commented Feb 12, 2024

extern is not required to declare a function, see https://trac.cppcheck.net/ticket/11555

I discussed that to introduce this more carefully.. but yes you are right..

@danmar danmar merged commit 0afe8a1 into danmar:main Feb 12, 2024
@olabetskyi olabetskyi deleted the ticket_12262 branch February 12, 2024 12:57
@chrchr-github
Copy link
Collaborator

This has caused a crash: https://trac.cppcheck.net/ticket/12432

@olabetskyi
Copy link
Collaborator Author

Fixed with #5978

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.

3 participants