common/snprintf.c: preserve wide integer values - #3598
Conversation
Keep fallback integer formatting at LLONG width. This prevents pointers and long-long values from being truncated through long on LLP64 systems. Handle the signed minimum without overflow and extend TEST_SNPRINTF with pointer and long-long regression comparisons. Fixes networkupstools#1602 AI assistance: OpenAI Codex gpt-5.6-sol was used for repository analysis, implementation, review, drafting and validation. The human contributor reviewed the change and remains responsible for it. Signed-off-by: user01010111 <lapses.50.booster@icloud.com>
|
A ZIP file with standard source tarball and another tarball with pre-built docs for commit 17548a3 is temporarily available: NUT-tarballs-PR-3598.zip. |
|
The CentOS 8/ppc64le OBS failure occurs during RPM database initialisation, before
CentOS 9/ppc64le builds this commit successfully. Could the CentOS 8/ppc64le preinstall image ( |
|
✅ Build nut 2.8.5.5161-master completed (commit ee81f6e695 by @)
|
|
✅ Build nut 2.8.5.5161-master completed (commit ee81f6e695 by @) |
|
OBS scenarios are on Open Build System, following their dependency tree etc. - as often as they regenerate them. I gather there is a regular inability to start the (emulated?) builders for this platform, which is mostly worked around by persistent restarting of the build in their Web-UI... |
jimklimov
left a comment
There was a problem hiding this comment.
I think the signed number support got broken here, the rest seems OK, thanks.
| if( value < 0 ) { | ||
| signvalue = '-'; | ||
| uvalue = -value; | ||
| uvalue = -uvalue; |
There was a problem hiding this comment.
This one change does not look right.
There was a problem hiding this comment.
Thanks, Jim. This is intentional unsigned arithmetic. After the negative LLONG value is converted to the corresponding unsigned type, C defines unary minus modulo 2^N, so this produces its magnitude without overflowing for LLONG_MIN. Using -value here would be undefined for LLONG_MIN. The added %lld comparison for LLONG_MIN exercises this path and passed against the system formatter.
Happy to add a short explanatory comment if that would make the intent clearer?
There was a problem hiding this comment.
I think yes, to avoid same questions later on. "Bits magic, not maths magic" :)
There was a problem hiding this comment.
Thanks, Jim. Fair point: unsigned negation in shared formatting code is unusual enough to deserve scrutiny. I've added a short comment so future readers do not have to reconstruct the modulo arithmetic to see why it is intentional.
Document why fmtint negates the unsigned magnitude when formatting negative values, including LLONG_MIN. AI assistance: OpenAI Codex gpt-5.6-sol was used for repository analysis, implementation, review, drafting and validation. The human contributor reviewed the change and remains responsible for it. Signed-off-by: user01010111 <lapses.50.booster@icloud.com>
| if (max < 0) | ||
| max = 0; | ||
|
|
||
| uvalue = value; |
There was a problem hiding this comment.
Still, I wonder how many platforms actually build this fallback implementations.
From experience with annoying compiler warnings, I'd actually expect this one with neither range-checks nor explicit casting to raise questions from static analysis.
Wouldn't it be clearer somewhat like this?
if (flags & DP_F_UNSIGNED) {
uvalue = (unsigned LLONG)value;
} else {
if (value < 0) {
signvalue = '-';
uvalue = -value;
} else {
uvalue = value;
if (flags & DP_F_PLUS) /* Do a sign (+/i) */
signvalue = '+';
else
if (flags & DP_F_SPACE)
signvalue = ' ';
}
}
Summary
LLONGwidth.%pand%ll*values from being truncated throughlongon LLP64 systems.LLONG_MINwithout signed overflow.TEST_SNPRINTFcomparisons for pointers, wide%lldvalues andLLONG_MIN.Fixes #1602
Validation
-std=c99 -Wall -Werror%p, wide%lldandLLONG_MINregression casesTEST_SNPRINTFcomparisonssnprintfandvsnprintfwere forced unavailable throughconfiguremake -C common V=1 snprintf.lopasses under-Werrormake stylecheck, the non-ASCII source check andgit diff --checkpassed.General C checklist
longmatches pointer or long-long width.printfandsprintfdeclarations remain isolated to the existingTEST_SNPRINTFharness; production logging behaviour is unchanged.common/snprintf.canddocs/developers.txtprecedent.AI assistance
OpenAI Codex gpt-5.6-sol was used for repository analysis, implementation, review, drafting and native/local validation. The human contributor reviewed the complete diff and validation evidence and remains responsible for the change.