On Wed, May 04, 2011 at 12:41:57PM -0700, Jeremy Huddleston wrote: > This is the one that I especially wanted to review on the list because it > doesn't sit well with me. > > Hitting the default switch-case in the first run through the loop results > in a NULL dereference (rbounds) when calling _XkbCheckBounds. In > subsequent runs, we get lucky in that rbounds was set in an earlier > iteration through the loop. I'm not really familiar enough with XKB to > know what is going on here. > > Should we be doing this: > rbounds= &tbounds;
I think this assigment is the better choice. AIUI, theoretically if we skip the check we may have a default doodad that's outside the section boundaries. Not that it matters much (it's geometry, it doesn't really matter anyway), but by using the above you at least clip it down to the allowed area. given that we haven't seen any actual issues yet, it suggests that this code is never hit by our current layouts. Cheers, Peter > or should we just be skipping the check (which is what I accidentally pushed)? > > http://people.freedesktop.org/~jeremyhu/analyzer/yuffie/20110504-0000/libX11/report-F0RlNS.html#EndPath > > On May 4, 2011, at 11:50, Jeremy Huddleston wrote: > > > > > XKBGeom.c:191:25: warning: Access to field 'x1' results in a dereference of > > a null pointer (loaded from variable 'rbounds') > > _XkbCheckBounds(bounds,rbounds->x1,rbounds->y1); > > ^~~~~~~ > > > > Found-by: clang static analyzer > > Signed-off-by: Jeremy Huddleston <jerem...@apple.com> > > --- > > src/xkb/XKBGeom.c | 4 ++-- > > 1 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/src/xkb/XKBGeom.c b/src/xkb/XKBGeom.c > > index e9e36d0..2365f48 100644 > > --- a/src/xkb/XKBGeom.c > > +++ b/src/xkb/XKBGeom.c > > @@ -147,7 +147,7 @@ register int i; > > XkbShapePtr shape; > > XkbRowPtr row; > > XkbDoodadPtr doodad; > > -XkbBoundsPtr bounds,rbounds=NULL; > > +XkbBoundsPtr bounds,rbounds; > > > > if ((!geom)||(!section)) > > return False; > > @@ -186,7 +186,7 @@ XkbBoundsPtr bounds,rbounds=NULL; > > default: > > tbounds.x1= tbounds.x2= doodad->any.left; > > tbounds.y1= tbounds.y2= doodad->any.top; > > - break; > > + continue; > > } > > _XkbCheckBounds(bounds,rbounds->x1,rbounds->y1); > > _XkbCheckBounds(bounds,rbounds->x2,rbounds->y2); > > -- > > 1.7.4.1 _______________________________________________ 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