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

Mark sqlite as lib folder in CodeQL scan #1293

Closed
wants to merge 1 commit into from

Conversation

ShrutiJaiswal1494
Copy link
Contributor

@ShrutiJaiswal1494 ShrutiJaiswal1494 commented Jul 18, 2024

Add CodeQL.yml file to add sqlite as a library.

@ShrutiJaiswal1494 ShrutiJaiswal1494 marked this pull request as ready for review July 19, 2024 20:11
@ShrutiJaiswal1494 ShrutiJaiswal1494 requested a review from a team as a code owner July 19, 2024 20:11
path_classifiers:
# Tagging the sqlite folder as 'library'.
library:
- "sqlite/**"
Copy link
Contributor

Choose a reason for hiding this comment

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

Will mark sqlite as 'library' exclude it from the scanning result?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I believe so.

Copy link
Contributor

Choose a reason for hiding this comment

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

But why exclude sqlite from the result? as we forked it, we need make sure the fork is clean on CodeQL, is this right?

@microsoft microsoft deleted a comment from ShrutiJaiswal1494 Jul 22, 2024
@lalitb
Copy link
Contributor

lalitb commented Jul 22, 2024

My opinion - I like the approach of configuring SQLite as a library for CodeQL, if that removes false positives. The usage of memcpy and memset within such a widely used database should generally be safe. Adding any guarded functions to make CodeQL happy could introduce side effects that might be difficult to catch in normal tests. A more fruitful effort would be to upgrade the SQLite3 snapshot to the latest version.

But anyway better to have @ThomsonTan decide on this, he knows more on CodeQL :)

p.s -

Not related, but an interesting usage spectrum for SQLite3 - https://www.sqlite.org/mostdeployed.html

SQLite is likely used more than all other database engines combined. Billions and billions of copies 
of SQLite exist in the wild.

it is seems likely that there are over one trillion (1e12) SQLite databases in active use.

@ThomsonTan
Copy link
Contributor

Based on our discussion, #1286 is preferred so close this one.

@ThomsonTan ThomsonTan closed this Aug 13, 2024
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