Fix deprecation warnings in macOS #59
Conversation
AI fixes so the logic might be off
There was a problem hiding this comment.
Pull request overview
This pull request addresses deprecation warnings in macOS by replacing unsafe sprintf calls with bounds-checked snprintf calls throughout the codebase. The changes also include minor corrections to format specifiers and removal of trailing whitespace.
Key changes:
- Replaced all
sprintfcalls withsnprintfto prevent buffer overflows - Updated buffer size calculations to include space for null terminators
- Changed format specifier from
%dto%ufor unsigned integer at line 1103 in filter_zoom.cpp - Removed trailing blank lines from multiple source files
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/metaproxy_prog.cpp | Replaced sprintf with snprintf using sizeof operator for PID logging |
| src/filter_zoom.cpp | Replaced multiple sprintf calls with snprintf, added buffer size calculations, but introduced two critical buffer overflow bugs due to incorrect variable usage in size calculations |
| src/filter_record_transform.cpp | Replaced sprintf with snprintf for error message formatting |
| src/filter_cgi.cpp | Replaced sprintf with snprintf using sizeof operator for content length |
| src/filter_backend_test.cpp | Replaced sprintf with snprintf using sizeof operator for record offset generation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
adamdickmeiss
left a comment
adamdickmeiss
left a comment
There was a problem hiding this comment.
It hurts.. 30 is plenty enough for formatting %d and that's with extra +1 . But in general this is just bad code, from my part. I will make an odr_printf .. function which can be used for all of this. It will be shorter and readable.
|
@adamdickmeiss yeah, all the malloc/snprintf bits should probably use a shared size variable rather than repeat the hard-coded sizes. Is there no way to have a wrapper for this thing or use some "safe" string concat? C coding hurts :) |
Co-authored-by: Adam Dickmeiss <adam@indexdata.dk>
This reverts commit 6974257.
adamdickmeiss
left a comment
adamdickmeiss
left a comment
There was a problem hiding this comment.
Some constructs would be simpler with nmem_printf.
@jakub-id can you see if this makes removes the warnings on macOS.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@adamdickmeiss will test today |
warnings now gone |
AI fixes so the logic might be off