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

[walter harms]

Am 15.08.2016 09:03, schrieb Hans de Goede:
> Hi,
> 
> On 13-08-16 19:53, Hans de Goede wrote:
>> 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 ?
> 
> Scrap that I just noticed the "It is a alternativ for free(3)." language
> you added as well as changing must into should. Please do not do that
> people must really always use XFree to free Xlib allocated resources.
> 
> Mixing / matching different alloc and free functions can end badly
> if the backend of one of them changes.
> 
> In this light I also suggest that you drop the reference to free(3) in
> the second sentence just make it. "If data is NULL, no operation is
> performed."
> 
> Regards,
> 
> Hans
> 
> 


--- a/man/XFree.man
+++ b/man/XFree.man
@@ -90,8 +90,9 @@ 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.
+You should use it to free any objects that were allocated by Xlib, unless
+an alternate function is explicitly specified for the object.
+
+If data is NULL, no operation is performed.



can we agree on this ?

re,
 wh