| Age | Commit message (Collapse) | Author |
|---|
|
Cleanup previous commit that littered MIN length clamping everywhere
%n was being used w/snprintf. Removed the %n usage altogether and
just clamps the return value out of snprintf to the buffer size minus
one for the null terminator.
The standard C library has such awful warts :/
|
|
It appears I overlooked that %n returns the length that would have
been printed regardless of the destination buffer size, not what
was actually written. The man page is misleading here:
n The number of characters written so far is stored into the
integer pointed to by the corresponding argument. That
argument shall be an int *, or variant whose size matches
the (optionally) supplied integer length modifier. No argu‐
ment is converted. (This specifier is not supported by the
bionic C library.) The behavior is undefined if the conver‐
sion specification includes any flags, a field width, or a
precision.
In testing, it isn't the count of what's actually written. It's
oblivious of truncated output scenarios where the output buffer has
been exhausted before reaching the %n. The man page should be
clarified here.
This commit does the simplest thing and simply clamps the length to
the destination buffer - 1 (for the \0). %n is being used to avoid
needing an strlen() in this somewhat hot path, but it might make
sense to instead use the snprintf return value similarly clamped
instead of %n since %n isn't doing what was expected.
|
|
|
|
This loop assumed ancestor->parent was !NULL, and that's not necessarily
always true. Due to these circular linked lists from the kernel's
list.h, they're not simply NULL delimited and we need the pointer to the
actual head to detect the end of the list. In libvmon, the head for the
siblings list is either the parent proc's children member, or the
processes member of the vmon struct. It may be more elegant to switch
to always having a root proc in libvmon, even if it's just synthetic,
for simplifying this crap. But for now, just determine which head is
relevant and check against it for loop termination.
Under some heavy parallel kernel compilations I was seeing occasional
vwm segfaults, and the addr2line of the ip in dmesg mapped to this
particular loop. I'm assuming the ancestor walk landed on a top-level
process and then this sibling detection tried dereferencing the
top-level proc's NULL parent and boom, segfault.
|
|
|
|
This was mixed up a bit in the cleanups... charts of !width represent the
uninitialized charts, so don't copy or free them instead of inhibiting
just the copy.
|
|
`make tags` in the linux kernel revealed that XDrawText can return
a BadLength error, which is not mentioned in the man page.
Glancing at the xorg-server source for doPolyText() this is found:
1192 else { /* print a string */
1193
1194 unsigned char *pNextElt;
1195
1196 pNextElt = c->pElt + TextEltHeader + (*c->pElt) * itemSize;
1197 if (pNextElt > c->endReq) {
1198 err = BadLength;
1199 goto bail;
1200 }
So there appears to bea fairly arbitrary ceiling on how many items one can
pass to XDrawText, and it probably depends on the cumulative length of the
individual items overflowing the maximum request length.
Well.. that's lame, and shrinking the maximum items makes it less likely to
trip over this in practice, but it probably just takes a long enough
individual item to trigger it again.
I had erred on the side of "excessively long" assuming XDrawText would just
deal and clip the text to the bounds of the destination drawable, just in
case there was an argv with lots of tiny items, then that would be covered.
This approach is incompatible with the potential for BadLength errors, so
drastically shrinking the maximum number of items until further notice.
|
|
|
