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.

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

Github's CodeQL returns way too many errors #129

Open
pguyot opened this issue Jan 9, 2022 · 3 comments
Open

Github's CodeQL returns way too many errors #129

pguyot opened this issue Jan 9, 2022 · 3 comments

Comments

@pguyot
Copy link
Owner

pguyot commented Jan 9, 2022

LGTM's company was acquired by GitHub and they later created more tests.
Some of these tests are actually founded and code should probably be fixed. Typically, the tool revealed some dead code.

There is a PR to run tests on every pull requests and pushes, even if it takes quite a long time (about one hour).
#127

Before merging it, we probably should fix the two critical alerts (usage of strcat that could overflow), as many other alerts as possible and decide on which tests to actually disable (for example we can disable the test reporting FIXME comments).
https://github.com/pguyot/Einstein/pull/127/checks?check_run_id=4752834281
https://github.com/pguyot/Einstein/security/code-scanning?page=1&query=is%3Aopen+pr%3A127

@MatthiasWM
Copy link
Collaborator

Thanks for adding this feature. Most (all?) of the FIXMEs are my fault, and as I mention in what became mit little todo list in #119, I will go through all FIXMEs and re document them and change them into TODOs, unless they are urgent bugs.

@MatthiasWM
Copy link
Collaborator

This is going to be interesting for the FLTK UI code. There is little use in deleting a button in a dialog box that is only allocated once and will live for the entire runtime of the app. Second, FLTK automatically delete all UI elements inside a window whenever the window is deleted. Explicitly deleting a button would be wrong. I will check how to suppress those false positives.

@pguyot
Copy link
Owner Author

pguyot commented Jan 9, 2022

The FIXMEs are not so much an issue, and I actually disabled the report on LGTM. We can agree on the semantic but just changing them to TODOs is probably not worth the effort. I noticed some of them are "incomplete" implementations, such as libffi not being ported to Windows & iOS.

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

No branches or pull requests

2 participants