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

[Hans de Goede]

Hi,

On 12-08-16 18:10, walter harms wrote:
>
>
> 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,

Looks good to me, can you submit this as a proper patch please ?

Thanks and Regards,

Hans


>
> --- 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 ();
>>>>>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
>