Fwd: Re: bug in XFree86 4.1.0 with Rage 128 driver?

Fwd: Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Kevin B. Hendricks]

[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]

Hi Michel (and FYI Herbert).

The bug was in XF 4.1.0 in Xrender.c not in OpenOffice

The patch is attached.  They (XF4 guys) simply missed changing a variable
name and ended up using random garbage in an "if"

Dan and Olaf, if you haven't gone to press with new releases, you probably
should include this one in your next respin builds of XFree86 4.1.0

Michel will you see that this makes it into the next XF 4.X release for us.

Thanks,

Kevin



> Will you please look at the following diff from Xrender.c in XF 4.0.2 to
> Xrender.c in XF410.  It looks very strange.  Starting in lie 190.
> It seems they no longer Xmalloc a value for xFormat (they now use xData)
> but just one line later they check if xFormat is zero.

> The code before that in XRenderQueryFormats() doesNOT set a value for
> xFormat at all after its is declared and before it is tested.

> I think someone just missed changing and xFormat to an xData unless I am
> missing something here.

> Kevin



>@@ -190,19 +191,20 @@
>               rep.numScreens * sizeof (xPictScreen) +
>               rep.numDepths * sizeof (xPictDepth) +
>               rep.numVisuals * sizeof (xPictVisual));
>-    xFormat = (xPictFormInfo *) Xmalloc (rlength);
>+    xData = (void *) Xmalloc (rlength);
>
>     if (!xri || !xFormat)
>     {
>        if (xri) Xfree (xri);
>-       if (xFormat) Xfree (xFormat);
>+       if (xData) Xfree (xData);
>        _XEatData (dpy, rlength);
>        UnlockDisplay (dpy);
>        SyncHandle ();
>        return 0;

[-- Attachment #2: xf410_render.patch --]
[-- Type: text/x-diff, Size: 340 bytes --]

--- xc/lib/Xrender/Xrender.c.prev	Wed Oct 10 22:38:25 2001
+++ xc/lib/Xrender/Xrender.c	Wed Oct 10 22:39:09 2001
@@ -193,7 +193,7 @@
 	       rep.numVisuals * sizeof (xPictVisual));
     xData = (void *) Xmalloc (rlength);
     
-    if (!xri || !xFormat)
+    if (!xri || !xData)
     {
 	if (xri) Xfree (xri);
 	if (xData) Xfree (xData);

Re: Fwd: Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Michel Dänzer]

"Kevin B. Hendricks" wrote:

> The bug was in XF 4.1.0 in Xrender.c not in OpenOffice
>
> The patch is attached.  They (XF4 guys) simply missed changing a variable
> name and ended up using random garbage in an "if"

Why does it work with the fbdev driver then?


> Michel will you see that this makes it into the next XF 4.X release for us.

I don't feel qualified to do that. Xpert@XFree86.Org is a mailing list which
the author of this code and other very competent XFree86 hackers read, or if
you're absolutely sure the fix is correct, please submit it to
fixes@XFree86.Org .


--
Earthling Michel Dänzer (MrCooper)    \   Debian GNU/Linux (powerpc) developer
CS student, Free Software enthusiast   \        XFree86 and DRI project member

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: Fwd: Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Olaf Hering]

On Thu, Oct 11, Michel Dänzer wrote:

> "Kevin B. Hendricks" wrote:
>
> > The bug was in XF 4.1.0 in Xrender.c not in OpenOffice
> >
> > The patch is attached.  They (XF4 guys) simply missed changing a variable
> > name and ended up using random garbage in an "if"
>
> Why does it work with the fbdev driver then?
>
>
> > Michel will you see that this makes it into the next XF 4.X release for us.
>
> I don't feel qualified to do that. Xpert@XFree86.Org is a mailing list which
> the author of this code and other very competent XFree86 hackers read, or if
> you're absolutely sure the fix is correct, please submit it to
> fixes@XFree86.Org .


keithp already put something in the CVS. It did not work with "ati", but
with fbdev (clgen).
The patch fixed it for me on the Pismo.



Gruss Olaf

--
 $ man clone

BUGS
       Main feature not yet implemented...

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Kevin Hendricks]

Hi All.

>> I don't feel qualified to do that. Xpert@XFree86.Org is a mailing list
>> which
>> the author of this code and other very competent XFree86 hackers read,
>> or if
>> you're absolutely sure the fix is correct, please submit it to
>> fixes@XFree86.Org .

Michel:

I just don't see how using an uninitialized variable in an "if"
statement can be anything else but a bug.  You could have at least
looked at the code in question before replying.  You are a member of
XFree representing ppc linux aren't you?  I assumed with your signature
and your earlier responses, you were the right person to contact.  If
not, I apologize for bothering you.

> keithp already put something in the CVS. It did not work with "ati", but
> with fbdev (clgen).
> The patch fixed it for me on the Pismo.

Thanks Olaf,

Glad to see it helped and is in CVS.

Kevin


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Kevin Hendricks]

Hi,

>> Why does it work with the fbdev driver then?

Just a guess.

Since we are using an uninitialized variable whatever was in memory
previously determined the value of xFormat.  So the result was random
behavior when checking for formats.

So anything that changed the stack usage or memory layout would / should
have an impact.

Kevin

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Olaf Hering]

On Thu, Oct 11, Michel Dänzer wrote:

> On Thu, 2001-10-11 at 14:53, Kevin Hendricks wrote:
>
> > >> I don't feel qualified to do that. Xpert@XFree86.Org is a mailing list
> > >> which the author of this code and other very competent XFree86 hackers read,
> > >> or if you're absolutely sure the fix is correct, please submit it to
> > >> fixes@XFree86.Org .
> >
> > Michel:
> >
> > I just don't see how using an uninitialized variable in an "if"
> > statement can be anything else but a bug.
>
> You're probably right. Never meant to say otherwise.
>
> > You could have at least looked at the code in question before replying.
>
> If I had enough time...
>
> > You are a member of XFree representing ppc linux aren't you?
>
> Yes, but I don't have any experience with this code, and I'm not member of
> the core team so I have to submit patches for inclusion as well. I was
> referring you to places I thought would allow for faster inclusion.
>
>
> > > keithp already put something in the CVS. It did not work with "ati", but
> > > with fbdev (clgen).
> > > The patch fixed it for me on the Pismo.
>
> So what's in CVS only works for fbdev, Kevin's patch is needed for r128?

Its clearly needed for everyone, Keith already did the commit a few
hours ago.



Gruss Olaf

--
 $ man clone

BUGS
       Main feature not yet implemented...

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Kevin Hendricks]

Hi Michel,

> So what's in CVS only works for fbdev, Kevin's patch is needed for r128?

I went and used cvsweb on Xfree86.org to see what keithp actually
committed.

The bug I found was actually *already* fixed by XF4 guys  soon after
XF410 was released (they simply removed the || !xFormat in the "if")
(given the dates on the Xrender.c changes).

So all keithp had to add was the code to check to make sure xData
malloced okay (i.e put back in || !xData in the "if").

So what is in cvs head right now for this code snippet turns out to be
exactly what my  patch will do for XF 4.1.0.

I assume the r128 versus framebuffer issue was only due to random memory
being tested so the results were/are random.

The patch is needed for all 4.1.0 respins since the bug only exists in
4.1.0 but anything based on cvs recently should be okay as is.

Hope this helps.

Kevin


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Michel Dänzer]

On Thu, 2001-10-11 at 17:37, Kevin Hendricks wrote:

> > So what's in CVS only works for fbdev, Kevin's patch is needed for
> > r128?
>
> I went and used cvsweb on Xfree86.org to see what keithp actually
> committed.
>
> The bug I found was actually *already* fixed by XF4 guys  soon after
> XF410 was released (they simply removed the || !xFormat in the "if")
> (given the dates on the Xrender.c changes).
>
> So all keithp had to add was the code to check to make sure xData
> malloced okay (i.e put back in || !xData in the "if").
>
> So what is in cvs head right now for this code snippet turns out to be
> exactly what my  patch will do for XF 4.1.0.
>
> I assume the r128 versus framebuffer issue was only due to random memory
> being tested so the results were/are random.
>
> The patch is needed for all 4.1.0 respins since the bug only exists in
> 4.1.0 but anything based on cvs recently should be okay as is.

The fix is only in the trunk, not the 4.1 branch?


--
Earthling Michel Dänzer (MrCooper)/ Debian GNU/Linux (powerpc) developer
XFree86 and DRI project member   /  CS student, Free Software enthusiast

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Kevin Hendricks]

Hi,

> The fix is only in the trunk, not the 4.1 branch?

I did not see it but perhaps cvsweb is not yet up to date.

I looked under xf-4_1-branch using cvsweb and it still needs to be
patched (with my patch).

Hope this helps,

Kevin

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/