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