[PATCH libXrandr 1/2] fix: doGetScreenResources() info: redundant null check on calling free()

[walter harms]

Am 12.08.2016 16:36, schrieb Hans de Goede:
> Hi,
> 
> On 12-08-16 16:09, walter harms wrote:
>>
>>
>> Am 12.08.2016 12:11, schrieb Hans de Goede:
>>> Hi,
>>>
>>> On 28-07-16 19:31, walter harms wrote:
>>>>
>>>> janitorial patch: remove some unneeded if() before free()
>>>
>>> This is not free() but Xfree() and "man Xfree" states:
>>>
>>> "A NULL pointer cannot be passed to this function."
>>>
>>
>> this is wrong.
>>
>> Xfree is a define for free()
>> Xlibint.h:#define Xfree(ptr) free((ptr))
> 
> Ah, yes then the man-page is wrong, my bad.
> 
>> more over the general use is this way.
>>
>> I will post a patch for the man-page.
>>
>> do you thing this is understandable ?
> 
> Maybe explicitly state that passing NULL
> is allowed and will do nothing ?
> 
> e.g. man 3 free has:
> 
> "If ptr is NULL, no operation is performed."
> 
> Regards,
> 

I did some emphasis on free since if someone has
a non-posix free() it would be a problem,

--- a/man/XFree.man
+++ b/man/XFree.man
@@ -90,8 +90,10 @@ Specifies the data that are to be freed.
 The
 .ZN XFree
 function is a general-purpose Xlib routine that frees the specified data.
-You must use it to free any objects that were allocated by Xlib,
-unless an alternate function is explicitly specified for the object.
-A NULL pointer cannot be passed to this function.
+It is a alternativ for free(3). You should use it to free any objects that
+were allocated by Xlib, unless an alternate function is explicitly specified
+for the object.
+
+Since it is based on free(3) if data is NULL, no operation is performed.
 .SH "SEE ALSO"
 \fI\*(xL\fP



> Hans
> 
> 
> 
>>
>> --- a/man/XFree.man
>> +++ b/man/XFree.man
>> @@ -90,8 +90,8 @@ Specifies the data that are to be freed.
>>  The
>>  .ZN XFree
>>  function is a general-purpose Xlib routine that frees the specified
>> data.
>> -You must use it to free any objects that were allocated by Xlib,
>> -unless an alternate function is explicitly specified for the object.
>> -A NULL pointer cannot be passed to this function.
>> +It is a alternativ for free(3). You should use it to free any objects
>> that
>> +were allocated by Xlib, unless an alternate function is explicitly
>> specified
>> +for the object.
>>  .SH "SEE ALSO"
>>  \fI\*(xL\fP
>>
>>
>>
>>> Regards,
>>>
>>> Hans
>>>
>>>
>>>>
>>>>
>>>> ---
>>>>  src/XrrScreen.c | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/XrrScreen.c b/src/XrrScreen.c
>>>> index f29071c..b8ce7e5 100644
>>>> --- a/src/XrrScreen.c
>>>> +++ b/src/XrrScreen.c
>>>> @@ -127,8 +127,8 @@ doGetScreenResources (Display *dpy, Window window,
>>>> int poll)
>>>>      xrsr = (XRRScreenResources *) Xmalloc(rbytes);
>>>>      wire_names = (char *) Xmalloc (rep.nbytesNames);
>>>>      if (xrsr == NULL || wire_names == NULL) {
>>>> -    if (xrsr) Xfree (xrsr);
>>>> -    if (wire_names) Xfree (wire_names);
>>>> +    Xfree (xrsr);
>>>> +    Xfree (wire_names);
>>>>      _XEatDataWords (dpy, rep.length);
>>>>      UnlockDisplay (dpy);
>>>>      SyncHandle ();
>>>>