[pduchi]

I don't have a Trac account, so there is no ticket for this; the details are below. Feel free to create one and I'll add it to the title.

#include <glib.h>

void f(double d)
{
    gchar buf[G_ASCII_DTOSTR_BUF_SIZE];
    g_ascii_formatd(buf, sizeof(buf), "%.2f", d);
    g_print("%s\n", buf);
}

$ cppcheck --library=gtk formatd.c
formatd.c:6:5: warning: g_ascii_formatd format string requires 1 parameter but only 0 are given. [wrongPrintfScanfArgNum]
formatd.c:6:21: warning: Uninitialized variable: buf [uninitvar]

g_ascii_formatd(buffer, buf_len, format, d) formats exactly one double into buffer.

• The double was configured as arg 4, so the format check did not count it. That entry is removed, and the format check now also checks its type (%d with a double is reported).
• buffer (arg 1, direction out) was marked not-uninit and strz although it is only written. It now has minsize buf_len instead, which also catches a buffer smaller than buf_len.

Tests: test/cfg/gtk.c covers the valid call, a wrong conversion and a too-small buffer. Both false positives show without the change; cfg/gtk.cfg validates against cppcheck-cfg.rng.

[dmcppcheck]

AI review

I found no real problems in this PR. Both false positives from the description go away: wrongPrintfScanfArgNum and uninitvar on the output buffer. Three checks still work: an uninitialized d is still reported, also when the format string is not a literal; a wrong conversion is reported (invalidPrintfArgType_*); and a buffer smaller than buf_len is reported (bufferAccessOutOfBounds).

One small regression: removing <arg nr="4"> also removes not-bool for d. With a literal format string the format check now covers this (invalidPrintfArgType_float). With a non-literal format string, passing a bool is no longer reported, where base reported invalidFunctionArgBool. That case is rare and I don't think it should block the PR.

Testing

Build and tests

• make -j8 cppcheck testrunner in pr/ built fine.
• ./testrunner TestLibrary TestBufferOverrun: 0 failures.
• test/cfg/gtk.c with the options from test/cfg/runtests.sh (--check-library --platform=unix64 --enable=style,information --inconclusive --force --check-level=exhaustive --inline-suppr --debug-warnings --library=gtk): no unexpected findings, and all the new inline suppressions are matched.
• I could not validate against the RelaxNG schema because xmllint is not installed here.

How I compared with base
I did not build base. Instead I ran the PR binary once with --library=gtk (new config) and once with --library=std --library=<base gtk.cfg> (old config), on scratch files with these cases:

• the valid call
• uninitialized d, with a literal format and with a non-literal one
• "%.2f %f" with one double
• %d with a double
• a buffer that is too small (stack and g_malloc, including char buf[38] with buf_len 39)
• a NULL buffer
• %g and %e
• a bool argument

Results

• PR: the only findings are the expected ones:
  • bufferAccessOutOfBounds for buf[38]/39, small[4]/39 and g_malloc(4)/39; no finding for buf[39]/39.
  • wrongPrintfScanfArgNum only for "%.2f %f", which says it needs 2 parameters and gets 1.
  • invalidPrintfArgType_sint and invalidPrintfArgType_float for the wrong type and the bool.
  • nullPointer for the NULL buffer.
  • uninitvar on d.
• Base config: every call gets wrongPrintfScanfArgNum ... only 0 are given, and every call gets uninitvar on the buffer.

Small regression

void g8(const char *fmt, int b) { char buf[39]; g_ascii_formatd(buf, 39, fmt, b==1); }

The base config reports Invalid g_ascii_formatd() argument nr 4. A non-boolean value is required. [invalidFunctionArgBool]; the PR config reports nothing.

Not detected by either config
buf+1 with a length of 39, and a buf_len held in a variable. These are limits of minsize, not something the PR changed.

---

Automated review of 395ef5e by Claude. The pull request was built and tested locally.

[pduchi]

AI review

I found no real problems in this PR. Both false positives from the description go away: wrongPrintfScanfArgNum and uninitvar on the output buffer. Three checks still work: an uninitialized d is still reported, also when the format string is not a literal; a wrong conversion is reported (invalidPrintfArgType_*); and a buffer smaller than buf_len is reported (bufferAccessOutOfBounds).

One small regression: removing <arg nr="4"> also removes not-bool for d. With a literal format string the format check now covers this (invalidPrintfArgType_float). With a non-literal format string, passing a bool is no longer reported, where base reported invalidFunctionArgBool. That case is rare and I don't think it should block the PR.

Testing

Build and tests

* `make -j8 cppcheck testrunner` in `pr/` built fine.

* `./testrunner TestLibrary TestBufferOverrun`: 0 failures.

* `test/cfg/gtk.c` with the options from `test/cfg/runtests.sh` (`--check-library --platform=unix64 --enable=style,information --inconclusive --force --check-level=exhaustive --inline-suppr --debug-warnings --library=gtk`): no unexpected findings, and all the new inline suppressions are matched.

* I could not validate against the RelaxNG schema because `xmllint` is not installed here.

How I compared with base I did not build base. Instead I ran the PR binary once with --library=gtk (new config) and once with --library=std --library=<base gtk.cfg> (old config), on scratch files with these cases:

* the valid call

* uninitialized `d`, with a literal format and with a non-literal one

* `"%.2f %f"` with one double

* `%d` with a double

* a buffer that is too small (stack and `g_malloc`, including `char buf[38]` with `buf_len` 39)

* a NULL buffer

* `%g` and `%e`

* a bool argument

Results

* **PR:** the only findings are the expected ones:
  
  * `bufferAccessOutOfBounds` for `buf[38]`/39, `small[4]`/39 and `g_malloc(4)`/39; no finding for `buf[39]`/39.
  * `wrongPrintfScanfArgNum` only for `"%.2f %f"`, which says it needs 2 parameters and gets 1.
  * `invalidPrintfArgType_sint` and `invalidPrintfArgType_float` for the wrong type and the bool.
  * `nullPointer` for the NULL buffer.
  * `uninitvar` on `d`.

* **Base config:** every call gets `wrongPrintfScanfArgNum ... only 0 are given`, and every call gets `uninitvar` on the buffer.

Small regression

void g8(const char *fmt, int b) { char buf[39]; g_ascii_formatd(buf, 39, fmt, b==1); }

The base config reports Invalid g_ascii_formatd() argument nr 4. A non-boolean value is required. [invalidFunctionArgBool]; the PR config reports nothing.

Not detected by either config buf+1 with a length of 39, and a buf_len held in a variable. These are limits of minsize, not something the PR changed.

Automated review of 395ef5e by Claude. The pull request was built and tested locally.

Thanks for this value feedback. Fixed the not-bool regression in 65c0efe: describing <arg nr="4"> again brings back "requires 1 parameter but only 0 are given" (the format check doesn't count described arguments), so the value is now described as <arg nr="variadic"> with <not-bool/>, as gnu.cfg does. Your bool case with a non-literal format is added to test/cfg/gtk.c and reports invalidFunctionArgBool again; the other cases are unchanged.

[dmcppcheck]

AI review

No new correctness problems. The not-bool regression is fixed with <arg nr="variadic">: invalidFunctionArgBool is reported again when the format string is not a literal. Both false positives from the description are gone.

One small difference from gnu.cfg: the new variadic entry has no <not-uninit/>. For a struct member that is not initialized, this drops uninitStructMember. The uninitvar warning on the same argument is still reported, so nothing is lost in practice.

As a side effect, the PR now reports an extra argument with a literal format ("%f", d, e), which base did not report.

Testing

Build and tests

• Built cppcheck and testrunner in pr/.
• ./testrunner TestLibrary: 0 failures.
• test/cfg/gtk.c with the options from runtests.sh (--inline-suppr --check-library --error-exitcode=1 ...): exit code 0, so every new suppression is matched.
• Not done: xmllint is not installed, so I could not validate against the schema.

How I compared with base: I ran the PR binary with --library=gtk and with base's gtk.cfg.

• Valid literal call: base gives wrongPrintfScanfArgNum ... only 0 are given and uninitvar on the buffer. The PR gives neither.
• g_ascii_formatd(buf, 39, "%f") with no value: both report wrongPrintfScanfArgNum.
• g_ascii_formatd(buf, 39, "%f", d, e): the PR reports requires 1 parameter but 2 are given. Base reported nothing.
• Non-literal format with a missing or an extra value: neither reports it. This is unchanged.
• Uninitialized d, arr[0] or s.x as the value: still reported as uninitvar.

Small difference without not-uninit on the variadic argument

struct S { double x; };
void b(const gchar *fmt) { struct S s; gchar buf[39]; g_ascii_formatd(buf, 39, fmt, s.x); g_print("%s", buf); }

Command: cppcheck -q --library=gtk --platform=unix64 u.c

• Base config: uninitvar s.x and uninitStructMember s.x.
• PR config: only uninitvar s.x.
• PR config with <not-uninit/> added to the variadic arg: both again, with no other change.

---

Automated review of 65c0efe by Claude. The pull request was built and tested locally.

[dmcppcheck]

AI review

I found no new problems. The current version (d9593a0) puts <not-uninit/> back on the variadic argument, which fixes the point from the last review. Both false positives from the description are gone. Uninitialized values, wrong conversions, bool values and buffers that are too small are still reported.

Testing

• Build: make -j8 cppcheck testrunner in pr/ succeeded. I did not build base; earlier reviews had already compared the PR with the base config.

• ./testrunner TestLibrary: 0 failures.

• test/cfg/gtk.c: I ran it with the runtests.sh options (--check-library --platform=unix64 --enable=style,information --inconclusive --force --check-level=exhaustive --inline-suppr --debug-warnings --library=gtk). Every new inline suppression matched and there were no unexpected findings. The one extra line in the output, unmatchedSuppression for missingIncludeSystem, came from a --suppress I had added to the command.

• Scratch file: I ran ./cppcheck -q --library=gtk --platform=unix64 --enable=style,warning --inconclusive a.c on these cases:

  • the call from the description with G_ASCII_DTOSTR_BUF_SIZE
  • using the return value
  • an uninitialized d
  • a g_malloc buffer
  • a non-literal format with a constant value

  The only findings were the expected uninitvar on d and an unrelated constVariablePointer style note. There was no wrongPrintfScanfArgNum and no uninitvar on the buffer.

• Not done: schema validation, because xmllint is not installed here.

---

Automated review of d9593a0 by Claude. The pull request was built and tested locally.