Qprintf Rewrite

[Lightning11wins]

qprintf.c had some code that confused me a lot. For example, there was a switch statement inside an if statement inside another switch statement inside a loop inside another if statement inside the else block of different if statement inside another if statement inside another loop... causing my brain to throw a stack overflow exception.

After understanding the code (which will be useful for the apos_autoscale proejct), I did some refactoring, and now it's much easier to read, in my opinion. Also, I added some tools for improved error handling, as usual.

GitHub Relationships

• Resolves Add %BOOL to qprintf() #162
• Resolves qprintf() prints % and & inside skipped conditionals #140
• Depends on Upgrade centrallix-lib Tests #170
  • Which depends on Add centrallix-lib Utilities #130

Greptile Summary

[Lightning11wins]

@greptileai Please review this PR.

[greptile-apps]

Greptile Summary

The PR substantially rewrites QPrintf and expands its tests, documentation, error reporting, and supporting library utilities.

• Refactors format parsing, conversion, quoting, growth, and overflow handling.
• Adds detailed QPrintf error tracking and extensive focused test coverage.
• Adds reusable checking, range, timer, string-formatting, and utility helpers.
• Updates the library build and parallel test infrastructure.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
centrallix-lib/src/qprintf.c	Rewrites QPrintf parsing and output handling; the previously reported predicate, initialization, no-grow, quote-boundary, syntax, and capacity issues are addressed at current HEAD.
centrallix-lib/include/qprintf.h	Expands the public QPrintf API and session error metadata to support the rewritten implementation.
centrallix-lib/tests/test_qprintf_69.c	Verifies that quoted overflow in a two-byte destination retains a quote and null terminator.
centrallix-lib/tests/test_qprintf_70.c	Covers the one-byte quoted-output boundary without permitting an out-of-bounds quote write.
centrallix-sysdoc/QPrintf.md	Documents QPrintf behavior and the expanded formatting interface.
centrallix-lib/Makefile.in	Adds new library objects and parallelized test and Valgrind execution infrastructure.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Parse format string] --> B[Resolve source specifier]
  B --> C[Apply length and conversion filters]
  C --> D{Quoted output?}
  D -- Yes --> E[Reserve and write opening quote]
  D -- No --> F[Translate or copy content]
  E --> F
  F --> G[Reserve and write closing quote]
  G --> H[Null-terminate destination]
  H --> I[Return copied length and session errors]

Loading

_{Reviews (17): Last reviewed commit: "Merge branch 'add-utils' into qprintf-re..." | Re-trigger Greptile}

[Lightning11wins]

@greptileai I think I resolved your comments. Please rereview. Are there any other concerns you have?

[Lightning11wins]

@greptileai I think I resolved your comments. Please rereview. Are there any other concerns you have?

[greptile-apps]

The root cause is that min_room = 2 only establishes the 2-byte reserve after Translate writes something. When no_grow is already true on entry, Translate's hard route short-circuits and writes nothing — dest_offset is unchanged, so the reserve is never created.

Instead of making the write conditional (which breaks your tests), clamp dest_offset right before the unconditional write:

			    no_grow = true;
			    }
			
			/** Write the closing quote, even if a buffer overflow occurred. **/
			/** We passed min_room = 2, so there will be enough space for it. **/
			/** However, if Translate wrote nothing (e.g. no_grow was already  **/
			/** true on entry), the reserve may not have been established — so  **/
			/** clamp dest_offset to guarantee room for quote + null terminator. **/
			if (dest_offset + 2 > *dest_size)
			    dest_offset = *dest_size - 2;
			(*dest)[dest_offset++] = quote;

The clamp is a no-op in the normal case — when Translate respected min_room = 2, dest_offset + 2 <= *dest_size already holds. It only activates when the buffer was already full on entry to the Translate call, ensuring the closing quote and null terminator always fit. This preserves the contract test_qprintf_51 and test_qprintf_52 rely on.

[Lightning11wins]

@greptileai I've made the changes you've requested but I haven't tested or pushed my code yet. Could you write a test case, following the same style established in test_qprintf_51.c that will ensure that will verify that this issue is fixed? Thus, it should fail on the previous implementation (the one that did not move the pointer AND the one that did not check no_grow), but it should pass on the current implementation that you showed me, which handles the situation correctly.

[greptile-apps]

Here's the test following the exact style of test_qprintf_51.c/test_qprintf_52.c.

Why this reproduces the bug: A 2-byte buffer for %STR&QUOT with input "ab" forces the exact path in question.

1. Opening quote written at buf[4], dest_offset = 1.
2. Translate writes nothing (no_grow triggered — no room for content + min_room = 2).
3. Closing-quote path: dest_offset + 2 = 3 > 2.
   • Without dest_offset clamp: closing quote written at buf[5], dest_offset = 2. Null-term guard 2 > 2 is false → no null written. buf[5] == '\''.
   • Without no_grow guard: grow_fn is called on a static buffer, returns false, falls through to same unconditional write → same result.
   • With the full fix: dest_offset clamped to *dest_size - 2 = 0, closing quote at buf[4], null at buf[5]. ✓

#include <stdio.h>
#include <unistd.h>
#include <fcntl.h>
#include <string.h>
#include <stdlib.h>
#include "qprintf.h"
#include <assert.h>

long long
test(char** tname)
    {
    int i, rval;
    int iter;
    unsigned char buf[16];

	*tname = "qprintf-53 %STR&QUOT with buffer too small for content (null termination)";
	iter = 200000;
	for(i=0;i<iter;i++)
	    {
	    buf[9]  = '\n';
	    buf[8]  = '\0';
	    buf[7]  = 0xff;
	    buf[6]  = 0xff;  /* sentinel: byte immediately after 2-byte buffer -- must not be clobbered */
	    /* buf+4 is the 2-byte buffer under test: occupies buf[4] and buf[5] */
	    buf[3]  = '\n';
	    buf[2]  = '\0';
	    buf[1]  = 0xff;
	    buf[0]  = '\0';
	    rval = qpfPrintf(NULL, buf+4, 2, "%STR&QUOT", "ab");
	    /* The 2-byte buffer must be null-terminated within its own bounds.
	     * Without dest_offset clamped before the closing-quote write,
	     * buf[5] holds '\'' and no null terminator is ever written. */
	    assert(buf[5] == '\0');
	    /* The byte immediately after the buffer must not be overwritten.
	     * Without the no_grow guard, an out-of-bounds null-terminator write
	     * could clobber this sentinel. */
	    assert(buf[6] == 0xff);
	    /* Bytes before the buffer must be untouched */
	    assert(buf[3] == '\n');
	    assert(buf[2] == '\0');
	    assert(buf[1] == 0xff);
	    assert(buf[0] == '\0');
	    /* Return value must indicate overflow: full output 'ab' + NUL = 5 bytes */
	    assert(rval == 5);
	    }

    return iter;
    }

Failure summary by variant:

Variant	buf[5] after call	assert(buf[5] == '\0')
No no_grow guard (original)	'\'' — closing quote, no null	FAILS
Guard added, no dest_offset clamp	'\'' — same	FAILS
Full fix (guard + clamp)	'\0'	passes

assert(buf[6] == 0xff) is a secondary tripwire: if a null terminator were written one byte past the buffer instead of being omitted, that sentinel catches it regardless.

One caveat: I inferred rval == 5 from the pattern in tests 51/52 (return value = bytes needed including NUL, for 'ab' that's 5). If the actual semantics differ, substitute assert(rval > 2).

[Lightning11wins]

@greptileai I think I resolved your comments, and I also added your test case with some clean up, bug fixing (on the return value), and other improvements. Please rereview. Are there any other concerns you have?