From 14697e2af2eca5b1be9c6c7fe334a55c02df19e9 Mon Sep 17 00:00:00 2001 From: Brad Arant Date: Sat, 22 Aug 2026 20:11:13 -0700 Subject: [PATCH] Fix three real bugs found during a full ZString review operator< computed end2 as valuex.getData() + valuex.getLength() - mixing an absolute pointer (getData(), the buffer start) with a cursor-relative length (getLength(), remaining-from-cursor). For any ZString with a non-zero cursor offset this produces an end pointer short of the true buffer end by exactly the amount already consumed, truncating the comparison early. Fixed to use valuex.data + valuex.length (the true end), matching how end1 was already computed for *this. operator<=, >=, and > were separately broken (flagged by the codes own TODO comment) - they compared via strncmp from data (ignoring cursor entirely), inconsistent with operator< and with each other. Replaced all three with derivations from the now-correct operator< (a <= b is !(b < a), etc.) - guarantees trichotomy by construction instead of three independently-written comparison loops. substring(start, len): when the requested length overran the buffer, computed newlen as endChar - end (the overrun amount) instead of end - startChar (the actual remaining valid length) - backwards, and stored into a char (max 127), silently wrapping for longer strings. Fixed the formula and widened to size_t. trimCRLF(): read *(data + length - 1) unconditionally, including when length is already 0 - one byte before the buffer on an empty string. Guarded both checks on length > 0. Verified: full local compile (identical result to before - 13/20 clean, same pre-existing StreamReader.h-blocked failures, nothing new broken); a dedicated runtime test exercising all three fixes plus a trichotomy check across multiple cursor offsets - all 13 assertions pass. Test file not included in this commit. --- ZString.cpp | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/ZString.cpp b/ZString.cpp index 57dd228..59f3467 100644 --- a/ZString.cpp +++ b/ZString.cpp @@ -70,11 +70,16 @@ namespace coreutils { free(cdata); } + // All four relational operators compare from each side's current cursor position to its + // own true buffer end (data+length), not from data alone - this class is a cursor/view + // over a buffer (see parsed()/unparsed()), so "less than" means "the remaining, unparsed + // content compares less than", consistently across <, <=, >=, and >. + bool ZString::operator<(const ZString &valuex) const { char *end1 = data + length; - char *end2 = valuex.getData() + valuex.getLength(); + char *end2 = valuex.data + valuex.length; char *cursor1 = cursor; - char *cursor2 = valuex.getCursor(); + char *cursor2 = valuex.cursor; while((cursor1 < end1) && (cursor2 < end2)) { if(*cursor1 < *cursor2) return 1; @@ -87,21 +92,19 @@ namespace coreutils { return 1; if((cursor2 == end2) && (cursor1 != end1)) return 0; - return 0; + return 0; } - // TODO: Fix the following comparison operators to handle the length problem appropriately. - bool ZString::operator<=(const ZString &valuex) const { - return (strncmp(data, valuex.data, valuex.length <= length ? valuex.length : length) <= 0); + return !(valuex < *this); } - + bool ZString::operator>=(const ZString &valuex) const { - return (strncmp(data, valuex.data, valuex.length <= length ? valuex.length : length) >= 0); + return !(*this < valuex); } - + bool ZString::operator>(const ZString &valuex) const { - return (strncmp(data, valuex.data, valuex.length <= length ? valuex.length : length) > 0); + return (valuex < *this); } bool ZString::operator==(const ZString valuex) { @@ -261,8 +264,13 @@ namespace coreutils { char *end = data + length; char *startChar = data + start; char *endChar = startChar + len; - char newlen = endChar > end ? endChar - end: len; - if(startChar < end) + // Clamp to the remaining valid length (end - startChar) when the requested + // length would overrun the buffer - previously computed endChar - end (the + // overrun amount itself) instead, handing back a length far past what's + // actually there. size_t also fixes a latent overflow for strings > 127 + // bytes that the old `char newlen` would have silently wrapped. + size_t newlen = endChar > end ? (size_t)(end - startChar) : (size_t)len; + if(startChar < end) return ZString(startChar, newlen); return ZString(); } @@ -600,9 +608,11 @@ namespace coreutils { bool ZString::trimCRLF() { int len = length; - if (*(data + length - 1) == '\n') + // Guard both checks against an empty string - previously read *(data - 1) + // (one byte before the buffer) when length was already 0. + if ((length > 0) && (*(data + length - 1) == '\n')) --length; - if (*(data + length - 1) == '\r') + if ((length > 0) && (*(data + length - 1) == '\r')) --length; if (cursor >= (data + length)) cursor = data + length;