[PULL] Minor bug-fixes discovered by static analysis.

Oliver McFadden oliver.mcfadden at nokia.com
Sun Mar 7 07:02:43 PST 2010


On Sat, 2010-03-06 at 19:19 +0100, ext Matt Turner wrote:
> On Fri, Mar 5, 2010 at 7:12 AM, Oliver McFadden
> <oliver.mcfadden at nokia.com> wrote:
> > are available in the git repository at:
> >
> >  git://gitorious.org/omcfadde/xserver.git analysis
> 
> Do you not have an account on FreeDesktop.org?

I do, but all of my Nokia X.org work is hosted on Gitorious. Shouldn't
be a problem though...

> > Oliver McFadden (5):
> >      exa: exaFinishAccess: Overrun of static array "pExaScr->access" of size 6 at position 6 with index variable "i"
> 
> This looks like it's got a typo in it. 'pPixmap),);' -> notice the extra comma.

No, this is correct. Check the define for EXA_FatalErrorDebugWithRet:

#ifdef DEBUG
#define EXA_FatalErrorDebug(x) FatalError x
#define EXA_FatalErrorDebugWithRet(x, ret) FatalError x
#else
#define EXA_FatalErrorDebug(x) ErrorF x
#define EXA_FatalErrorDebugWithRet(x, ret) \
do { \
    ErrorF x; \
    return ret; \
} while (0)
#endif

So it will evaluate as: { FatalError(...); return ; }

> >      fb: fbFinishScreenInit: leaked_storage: Variable "(visuals|depths)" goes out of scope
> 
> Reviewed-by: Matt Turner <mattst88 at gmail.com>
> 
> >      parser: xf86readConfigFile: unreachable: This code cannot be reached: "free(val.str);"
> 
> Not sure I understand this one.

Error() was called before free(), thus free() is unreachable code, and
val.str will never be freed and NULL-ed.

> >      common: xf86Configure: alloc_strlen: Allocated memory does not have space for the terminating NUL of the string
> 
> Reviewed-by: Matt Turner <mattst88 at gmail.com>
> 
> >      Xext: IdleTimeBlockHandler: unsigned_compare: Comparing unsigned less than zero is never true. "timeout < 0UL"
> 
> I'm not sure this does it exactly. Up at line 2320, we have 'unsigned
> long timeout = -1;'.

Yes, you're right this one needs further investigation. However if you
could check the ones that I've explained above and add your Reviewed-by
tags, I can send a new pull request with the patches.

I'll also check into the unsigned_compare patch further.

> 
> >
> >  Xext/sync.c                       |    4 +---
> >  exa/exa.c                         |    4 ++--
> >  fb/fbscreen.c                     |    4 ++++
> >  hw/xfree86/common/xf86Configure.c |    2 +-
> >  hw/xfree86/parser/read.c          |    4 ++--
> >  5 files changed, 10 insertions(+), 8 deletions(-)
> 
> Matt Turner

Thanks,
-- Oliver.




More information about the xorg-devel mailing list