Fix DATA_FORMAT printf/scanf for 32-bit architectures with 64-bit off_t#411
Open
daichifukui wants to merge 1 commit into
Open
Fix DATA_FORMAT printf/scanf for 32-bit architectures with 64-bit off_t#411daichifukui wants to merge 1 commit into
daichifukui wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the handling of file sizes across the codebase to use unsigned long long (ull_size) when parsing database records, preventing potential overflow and formatting issues. However, several review comments point out that the size variable (of type off_t) has become redundant in multiple functions (such as do_dump_db, handle_record, main in trustdb_format_test.c, and test_data_format_round_trip) and can be safely removed. Additionally, casting ull_size to off_t on 32-bit systems without _FILE_OFFSET_BITS=64 can cause sign extension issues when printing, which can be avoided by using ull_size directly.
On 32-bit architectures built with _FILE_OFFSET_BITS=64, off_t is 64-bit but size_t is 32-bit. Using %zu for an off_t field causes scanf to write only 4 bytes into an 8-byte off_t, corrupting the upper half. Switch to %llu with explicit casts throughout.
daichifukui
force-pushed
the
dfukui/fix-data-format
branch
from
af7f790 to
a62e190
Compare
Member
|
32 bit architecture is not a priority for this project. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On 32-bit architectures where _FILE_OFFSET_BITS=64 is defined (e.g. by a distribution's build system), off_t is 64-bit while size_t remains 32-bit. The trust database format macros DATA_FORMAT and DATA_FORMAT_IN used %zu, which expects a size_t-sized argument. Passing off_t values with %zu causes two distinct problems:
This fix changes DATA_FORMAT and DATA_FORMAT_IN to use %llu, and updates all call sites to cast file size values to unsigned long long for printf-family calls and to use an intermediate unsigned long long ull_size variable for scanf-family calls before assigning back to off_t.
This issue was originally identified as a build failure (FTBFS) on armhf in Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1132273. The root cause is that Debian injects -D_FILE_OFFSET_BITS=64 via dpkg-buildflags for all packages on 32-bit architectures. Any other distribution or build environment that defines _FILE_OFFSET_BITS=64 on a 32-bit target will encounter the same issue, so this fix is beneficial beyond Debian.