On (14/04/16 12:47), Sumit Bose wrote:
>On Thu, Apr 14, 2016 at 12:21:50PM +0200, Lukas Slebodnik wrote:
>> On (14/04/16 10:39), Pavel Reichl wrote:
>> >On 04/14/2016 10:28 AM, Lukas Slebodnik wrote:
>> >>ehlo,
>> >>
>> >>@see commit message in attached trivial patch.
>> >>
>> >>LS
>> >>
>> >>
>> >Hello,
>> >
>> >patch does not apply, code in patch doesn't look like recent master to 
>> >me...?
>> >
>> Patch was from latest 1.13 branch.
>> This issue is fixed in master branch in similar way.
>> 
>> The main dfference is that secondary_name is not checked for NULL
>> before releasing memory. But checking for NULL is not done everywhere
>> in the idmap code. So it needn't be changed.
>
>I assumed that modern free()-like functions will be able to handle NULL,
>but you are right, since it is configurable this expectation should be
>spelled out. Do you think a patch like
>
>diff --git a/src/lib/idmap/sss_idmap.h b/src/lib/idmap/sss_idmap.h
>index 483241e..8a09296 100644
>--- a/src/lib/idmap/sss_idmap.h
>+++ b/src/lib/idmap/sss_idmap.h
>@@ -135,10 +135,11 @@ struct dom_sid;
>  * @brief Initialize idmap context
>  *
>  * @param[in] alloc_func Function to allocate memory for the context, if
>- *                       NULL malloc() id used
>+ *                       NULL malloc() is used
ACK to typo :-)

>  * @param[in] alloc_pvt  Private data for allocation routine
>- * @param[in] free_func  Function to free the memory the context, if
>- *                       NULL free() id used
>+ * @param[in] free_func  Function to free the memory allocated by alloc_func, 
>if
>+ *                       NULL free() is used, free_func() must handle NULL as
>+ *                       input gracefully

ACK ++
BTW alternative would be to use sss_idmap_free_ptr instad of ctx->free_func.
This internal function is already used on some places.

LS
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to