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

[Eirik Byrkjeflot Anonsen]

Alan Coopersmith <alan.coopersmith at sun.com> writes:

> Eirik Byrkjeflot Anonsen wrote:
>> Alan Coopersmith <alan.coopersmith at sun.com> writes:
>>
>>> 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..
>>> 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.)
>

However, XFree() is not xorg-specific (since you're supposed to call
it after calling e.g. XQueryTree).  And yes, it would be safe to
provide better guarantees than the specification, except that it may
cause people to rely on it.  Which is bad if it is only "accidentally"
true.  Given that there are other x servers than xorg (or even
potentially older versions with different behaviour), I do not
consider this completely safe.

(ok, xfree and XFree aren't exactly the same function, but they are
similar enough that habits with one will carry over to the other.)

None the less, I am all for convincing all the xlib providers to make
their XFree() "null-safe", and note in the specification that "new
enough" xlibs can be expected to support XFree(NULL).  It does seem
rather like the "right thing".

eirik