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

Use parametetrized SQL statement to allow special chars #26

Merged

Conversation

mromerdagan
Copy link
Contributor

Moving to parametetrized SQL statement allows using chars like " ' " in notes

@mromerdagan mromerdagan force-pushed the param-sql-stmnt branch 2 times, most recently from 2314786 to e7b8918 Compare January 27, 2022 08:16
@TicklishHoneyBee
Copy link
Owner

Hi, so this is a great idea, but there's a couple of issues with it as is.

Firstly, if statements with only a single line body (and no else) don't needed braces. You've added a bit of a mix and match of this so please remove some braces (most notably, around line 316 of db.c). Also some spaces were added around an '}else{' making it '} else {' and different to every other else in the project.

Secondly, you've used sqlite3_errmsg directly inside a printf in a couple of places, please either read directly from db_data.error_msg (if set), or use the included wrapper function db_err which prevents a NULL return that can cause segfaults in some printf implementations. Also, being system/internal error messages, these should use fprintf to output to stderr rather than stdout.

@mromerdagan
Copy link
Contributor Author

Hi @TicklishHoneyBee , thanks for your reply. I will follow your guidelines and perform additional commit.

Move to parametetrized SQL statement allows using chars like "'" in
notes and also avoid sql injection
@mromerdagan
Copy link
Contributor Author

Hi @TicklishHoneyBee , I followed your guidelines in my last commit.

@TicklishHoneyBee TicklishHoneyBee merged commit 2481904 into TicklishHoneyBee:master Mar 23, 2022
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.

None yet

2 participants