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.
This commit is contained in:
Brad Arant committed 2026-08-22 20:11:13 -07:00
1 parent 4082e82466
commit 14697e2af2
1 file changed
+20 -10
+20 -10
View File
@@ -70,11 +70,16 @@ namespace coreutils {
free(cdata); 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 { bool ZString::operator<(const ZString &valuex) const {
char *end1 = data + length; char *end1 = data + length;
char *end2 = valuex.getData() + valuex.getLength(); char *end2 = valuex.data + valuex.length;
char *cursor1 = cursor; char *cursor1 = cursor;
char *cursor2 = valuex.getCursor(); char *cursor2 = valuex.cursor;
while((cursor1 < end1) && (cursor2 < end2)) { while((cursor1 < end1) && (cursor2 < end2)) {
if(*cursor1 < *cursor2) if(*cursor1 < *cursor2)
return 1; return 1;
@@ -90,18 +95,16 @@ namespace coreutils {
return 0; return 0;
} }
// TODO: Fix the following comparison operators to handle the length problem appropriately.
bool ZString::operator<=(const ZString &valuex) const { 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 { 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 { 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) { bool ZString::operator==(const ZString valuex) {
@@ -261,7 +264,12 @@ namespace coreutils {
char *end = data + length; char *end = data + length;
char *startChar = data + start; char *startChar = data + start;
char *endChar = startChar + len; char *endChar = startChar + len;
char newlen = endChar > end ? endChar - end: len; // 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) if(startChar < end)
return ZString(startChar, newlen); return ZString(startChar, newlen);
return ZString(); return ZString();
@@ -600,9 +608,11 @@ namespace coreutils {
bool ZString::trimCRLF() { bool ZString::trimCRLF() {
int len = length; 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; --length;
if (*(data + length - 1) == '\r') if ((length > 0) && (*(data + length - 1) == '\r'))
--length; --length;
if (cursor >= (data + length)) if (cursor >= (data + length))
cursor = data + length; cursor = data + length;