[PATCH 2/6] xserver: Possible memory leaks, stricter option checks, UnInit (NewInputDeviceRequest)
jbarnes at virtuousgeek.org
Fri Mar 30 14:04:08 PDT 2007
On Friday, March 30, 2007, Magnus Vigerlöf wrote:
> On Friday 30 March 2007 19:26, Alan Coopersmith wrote:
> > Eirik Byrkjeflot Anonsen wrote:
> > > Alan Coopersmith <alan.coopersmith at sun.com> writes:
> > >> And ANSI C requires free(NULL) to be a safe no-op anyways, so it's
> > >> not needed at all on modern OS'es.
> > >
> > > But I don't think xfree(NULL) is guaranteed to be safe. And I doubt
> > > that xfree(NULL) is specified as being "as safe as" free(NULL).
> > We define xfree(), so what stops us from raising xfree(NULL) from
> > "Undefined" to "Guaranteed safe no-op"? All we have to do is
> > document it - it won't break anything that works already. (If you
> > want to kill the process, call abort(), not Xfree(NULL) and hope it
> > segfaults.)
> I don't have any problem to start crawl through the code and start
> fixing these kinds of things. But shouldn't we address all the different
> naming schemes and implementation around malloc (et al) at the same
> time? For malloc I've found the following defines/functions so far in
> xserver: xalloc, xnfalloc, Xalloc, __glXMalloc, XtMalloc, and
> xf86confmalloc. Some are needed, but not all for sure... Eliminate all
> but xalloc, Xalloc, xnf*, and XNF*? Maybe even the ones starting with
> 'x' as well?
Yes please. It would be great if we could just use the C library routines
> Is there a macro that set the freed pointer to NULL after freeing the
> memory somewhere? I'd prefer a clean SEGV instead of an obscure error
> that is caused by a stray pointer... And if that can be made in a simple
> manner in the code I'd be happy. XfreeZ(ptr) ?
If we free memory twice (as opposed to freeing NULL pointers) we probably
want to catch that rather than have the second free be a no-op (as it
would be if free set the pointer to NULL). Usually that means turning on
some sort of malloc/free checking though...
More information about the xorg