[PATCH 2/6] xserver: Possible memory leaks, stricter option checks, UnInit (NewInputDeviceRequest)

[Eirik Byrkjeflot Anonsen]

Jesse Barnes <jbarnes at virtuousgeek.org> writes:

> On Thursday, March 29, 2007, Magnus Vigerlöf wrote:
>> On Thursday 29 March 2007 04:40, Eric Anholt wrote:
>> [...]
>>
>> > > I guess xfree can't handle freeing already NULL pointers?  If so it
>> > > could cleanup error paths like this a little.  Looks good though.
>> >
>> > The first thing Xfree (what xfree() is) does is check if the pointer
>> > is null and return if so.
>>
>> You're right. Xfree (and free as well) does seem to handle this. However
>> many places this check is made in the code anyway, and there's a comment
>> complaining about this 0-pointer check in Xfree as well..
>>
>> Where do we want to have this check?
>
> I think having it in free/xfree is nicer, since it can make error path code 
> a little nicer.  Better one conditional in free/xfree than dozens all over 
> the tree.
>

However, I do believe the documentation says "A NULL pointer cannot be
passed to this function."  While it is "safe" to add the check to
xfree(), it is not safe to remove the test around calls to it, unless
you know that you're using a "checking" xfree().

I guess you could argue that all code that is only used in xorg is
allowed to assume a "checking" xfree().  But you have to figure out
how to make sure that coders writing for X11 (i.e. not necessarily
xorg) does not get lazy and forget to check their pointers.

On the other hand, given that free() is documented to check for null
pointers, it would make sense to change the specification of xfree()
to check as well (I know I always forget to check my pointers before
calling xfree).  Though it would mean that correct programs according
to the new spec would segfault when run against correct xlibs
according to the old spec.

eirik