[PULL] Bugzilla triage, take two
Corbin Simpson
mostawesomedude at gmail.com
Fri Mar 26 15:10:49 PDT 2010
On Fri, Mar 26, 2010 at 11:43 AM, Adam Jackson <ajax at nwnk.net> wrote:
> On Fri, 2010-03-26 at 06:54 -0700, Corbin Simpson wrote:
>
>> Rami Ylimaki (1):
>> os: Prevent backtrace from being stopped in noreturn functions.
>>
>> are available in the git repository at:
>>
>> git://anongit.freedesktop.org/~csimpson/xserver triage
>>
>> Corbin Simpson (1):
>> dix: Fix a small potential memory leak in ProcRotateProperties. (#25216)
>
> Ack.
>
>> David Jander (1):
>> kdrive: Fix incomplete rotation matrix. (#20941)
>
> Looks plausible, although I don't really understand the rotation code.
>
>> Evgeny M. Zubok (1):
>> xfree86: Change VBE version early-out to 1.2. (#22672)
>
> Ack, although there's no real reason for that version check at all. If
> the VBE call isn't present we'll fail no matter what.
>
>> Ilpo Ruotsalainen (1):
>> shadow: Optimize shadowUpdatePacked(). (#26973)
>
> Nack. Read the code (abridged):
>
>> while (h--)
>> {
>> // ...
>> sha = shaLine;
>> while (width) {
>> // ...
>> win = winBase + (scr - scrBase);
>> // ...
>> while (i--)
>> *win++ = *sha++;
>> }
>> shaLine += shaStride;
>> y++;
>> }
>
> The proposed replacement for that while (i--) is
>
> memcpy(win, sha, i * sizeof(FbBits));
>
> But that's not equivalent; in the memcpy version, sha never moves. So
> if you ever go around the while (width) loop more than once, you'll be
> moving the destination of the memcpy, but not the source. I think you
> want:
>
> memcpy(win, sha, i * sizeof(FbBits));
> sha += i;
>
>> Joe Rabinoff (1):
>> xfree86: Fix off-by-one math in screen rotation. (#20762)
>
> Again, looks plausible.
>
>> TSUKAHARA Ken (1):
>> dix: Prevent denial-of-color attack. (#9356)
>
> Nack in its current form at least. These hunks are bogus:
>
>> --- a/dix/colormap.c
>> +++ b/dix/colormap.c
>> @@ -811,6 +811,7 @@ AllocColor (ColormapPtr pmap,
>> VisualPtr pVisual;
>> int npix;
>> Pixel *ppix;
>> + int initialPixels = pmap->numPixelsRed[client];
>>
>> pVisual = pmap->pVisual;
>> (*pmap->pScreen->ResolveColor) (pred, pgreen, pblue, pVisual);
>> @@ -970,8 +971,15 @@ AllocColor (ColormapPtr pmap,
>> }
>> pcr->mid = pmap->mid;
>> pcr->client = client;
>> - if (!AddResource(FakeClientID(client), RT_CMAPENTRY, (pointer)pcr))
>> - return (BadAlloc);
>> + if (class == PseudoColor) {
>> + if (initialPixels == 0) {
>> + if (!AddResource(FakeClientID(client), RT_CMAPENTRY, (pointer)pcr))
>> + return (BadAlloc);
>> + }
>> + } else {
>> + if (!AddResource(FakeClientID(client), RT_CMAPENTRY, (pointer)pcr))
>> + return (BadAlloc);
>> + }
>> }
>> return (Success);
>> }
>
> In the second hunk, we're inside a conditional on
>
> if ((pmap->numPixelsRed[client] == 1) && /* ... */
>
> And, if we're PseudoColor, we don't increment numPixelsRed[client]
> between these two hunks. So the if (initialPixels == 0) conditional is
> really if (0). Which is definitely wrong; if a client allocates a color
> in a pseudocolor map, that color needs to be GC'd at client death.
>
> The rest of it seems to be changing ->refcnt in EntryPtr from "count of
> how many times this color has been allocated" to "count of how many
> clients have allocated this color", which I'm pretty sure would need to
> be reflected in more places than just FindColor.
>
> - ajax
>
Alrighty. I've closed the denial-of-color bug based on your comments
and the general age and silliness of the code, and dropped that patch.
I went ahead and split the branch into bugfixes and triage; please
only pull from bugfixes.
~ C.
The following changes since commit 579715f830fbbca9e1ecb17dc18176132f5969e7:
Rami Ylimaki (1):
os: Prevent backtrace from being stopped in noreturn functions.
are available in the git repository at:
git://anongit.freedesktop.org/~csimpson/xserver bugfixes
Corbin Simpson (1):
dix: Fix a small potential memory leak in ProcRotateProperties. (#25216)
Evgeny M. Zubok (1):
xfree86: Change VBE version early-out to 1.2. (#22672)
dix/property.c | 6 ++++--
hw/xfree86/vbe/vbe.c | 2 +-
2 files changed, 5 insertions(+), 3 deletions(-)
--
When the facts change, I change my mind. What do you do, sir? ~ Keynes
Corbin Simpson
<MostAwesomeDude at gmail.com>
More information about the xorg-devel
mailing list