Re: [PATCH] Fix crash when all glyphs of a given depth are freed, but not all glyphsets

2010-03-24 Thread Michel Dänzer 
On Wed, 2010-03-24 at 13:03 +0100, Keith Packard wrote: 
> On Tue, 23 Mar 2010 12:39:30 -0400, Peter Harris  wrote:
> > On 2010-03-23 12:26, Dan Nicholson wrote:
> > > I think Keith usually looks for an explicit CC to imply that patches
> > > are ready to be applied.
> > 
> > I did CC: Keith.
> 
> I'm just travelling; anything Cc'd to me ends up in my 'patchq' mailbox
> and gets applied unless I see dissent about it, sometimes it just takes
> a day or two if I'm stuck in an airplane or meetings.

I suspect the confusion arose from the fact that posts sent out from the
list don't show you in CC: even if the sender put you there. I think
this is probably due to something in your list subscription settings.


-- 
Earthling Michel Dänzer   |http://www.vmware.com
Libre software enthusiast |  Debian, X and DRI developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Fix crash when all glyphs of a given depth are freed, but not all glyphsets

2010-03-24 Thread Keith Packard 
On Tue, 23 Mar 2010 12:39:30 -0400, Peter Harris  wrote:
> On 2010-03-23 12:26, Dan Nicholson wrote:
> > I think Keith usually looks for an explicit CC to imply that patches
> > are ready to be applied.
> 
> I did CC: Keith.

I'm just travelling; anything Cc'd to me ends up in my 'patchq' mailbox
and gets applied unless I see dissent about it, sometimes it just takes
a day or two if I'm stuck in an airplane or meetings.

-- 
keith.pack...@intel.com


pgp3ppbwmc9Rv.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Fix crash when all glyphs of a given depth are freed, but not all glyphsets

2010-03-23 Thread Peter Harris 
On 2010-03-23 12:26, Dan Nicholson wrote:
> I think Keith usually looks for an explicit CC to imply that patches
> are ready to be applied.

I did CC: Keith.

I also added the note as per http://www.x.org/wiki/XServer "If a patch
should be merged to master directly instead of going through a personal
tree and a pull request, make sure you state so in the patch email and
the release manager is on the CC list."

Is the wiki out of date?

Thanks,
 Peter Harris
-- 
   Open Text Connectivity Solutions Group
Peter Harrishttp://connectivity.opentext.com/
Research and DevelopmentPhone: +1 905 762 6001
phar...@opentext.comToll Free: 1 877 359 4866
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Fix crash when all glyphs of a given depth are freed, but not all glyphsets

2010-03-23 Thread Dan Nicholson 
I think Keith usually looks for an explicit CC to imply that patches
are ready to be applied.

On Tue, Mar 23, 2010 at 9:08 AM, Peter Harris  wrote:
> This is how the crash can be triggered with only two clients on the system:
> Client A: (already running)
> Client B: Connect
> Client B: CreateGlyphSet(depthN)
> Client A: Disconnect
> Server: free globalGlyphs(depthN)
> Client B: AddGlyphs(depthN)
> Server: SEGV
>
> This crash was introduced with the FindGlyphsByHash function
> in 516b96387b0e57b524a37a96da22dbeeeb041712. Before that revision,
> ResizeGlyphSet was always called before FindGlyphRef, which would
> re-create globalGlyphs(depthN) if necessary.
>
> X.Org Bug 20718 
>
> Reviewed-by: Adam Jackson 
> Signed-off-by: Peter Harris 
> ---
>
> Keith,
>  Please apply directly. I haven't set up a tree that can be pulled
> from yet.
>
>  render/glyph.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/render/glyph.c b/render/glyph.c
> index 0b864ad..f0f3b19 100644
> --- a/render/glyph.c
> +++ b/render/glyph.c
> @@ -217,6 +217,9 @@ FindGlyphByHash (unsigned char sha1[20], int format)
>     GlyphRefPtr gr;
>     CARD32 signature = *(CARD32 *) sha1;
>
> +    if (!globalGlyphs[format].hashSet)
> +       return NULL;
> +
>     gr = FindGlyphRef (&globalGlyphs[format],
>                       signature, TRUE, sha1);
>
> --
> 1.7.0.2
>
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Fix crash when all glyphs of a given depth are freed, but not all glyphsets

2010-03-23 Thread Adam Jackson 
On Mon, 2010-03-22 at 15:20 -0400, Peter Harris wrote:

> This crash was introduced with the FindGlyphsByHash function
> in 516b96387b0e57b524a37a96da22dbeeeb041712. Before that revision,
> ResizeGlyphSet was always called before FindGlyphRef, which would
> re-create globalGlyphs(depthN) if necessary.
> 
> Signed-off-by: Peter Harris 

Reviewed-by: Adam Jackson 

- ajax


signature.asc
Description: This is a digitally signed message part
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Fix crash when all glyphs of a given depth are freed, but not all glyphsets

2010-03-23 Thread Michel Dänzer 
On Mon, 2010-03-22 at 15:20 -0400, Peter Harris wrote: 
> This is how the crash can be triggered with only two clients on the system:
> Client A: (already running)
> Client B: Connect
> Client B: CreateGlyphSet(depthN)
> Client A: Disconnect
> Server: free globalGlyphs(depthN)
> Client B: AddGlyphs(depthN)
> Server: SEGV
> 
> This crash was introduced with the FindGlyphsByHash function
> in 516b96387b0e57b524a37a96da22dbeeeb041712. Before that revision,
> ResizeGlyphSet was always called before FindGlyphRef, which would
> re-create globalGlyphs(depthN) if necessary.
> 
> Signed-off-by: Peter Harris 

Looks like this could fix
http://bugs.freedesktop.org/show_bug.cgi?id=20718 . If so, please
reference the bug report in the commit message, resolve it when the fix
lands, and maybe nominate it for server-1.7-branch.


> ---
>  render/glyph.c |3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/render/glyph.c b/render/glyph.c
> index 0b864ad..f0f3b19 100644
> --- a/render/glyph.c
> +++ b/render/glyph.c
> @@ -217,6 +217,9 @@ FindGlyphByHash (unsigned char sha1[20], int format)
>  GlyphRefPtr gr;
>  CARD32 signature = *(CARD32 *) sha1;
>  
> +if (!globalGlyphs[format].hashSet)
> + return NULL;
> +
>  gr = FindGlyphRef (&globalGlyphs[format],
>  signature, TRUE, sha1);
>  


-- 
Earthling Michel Dänzer   |http://www.vmware.com
Libre software enthusiast |  Debian, X and DRI developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel