On 12/03/2014 05:58 PM, Jakub Hrozek wrote:
On Wed, Dec 03, 2014 at 04:33:33PM +0100, Pavel Březina wrote:
>> - if (groupnames[i] == NULL) {
>> + tmpstr = sss_replace_space(groupnames, name,
>> +
ireq->ifp_ctx->rctx->override_space);
>
> Sure, you can allocate tmpstr on groupnames, I think ireq or tmp_ctx would
> be better choice.
>
>> + if (tmpstr == NULL) {
>> DEBUG(SSSDBG_MINOR_FAILURE, "Cannot normalize %s\n",
name);
>> continue;
>> }
>> } else {
>> - groupnames[i] = name;
>> + tmpstr = name;
>> }
>> +
>> + groupnames[i] = sss_get_cased_name(ireq, tmpstr,
domain->case_preserve);
>
> Here, groupnames should be used as talloc context instead of ireq.
> sss_get_cased_names duplicates the string so tmpstr should be freed.
>
>> + if (groupnames[i] == NULL) {
>> + DEBUG(SSSDBG_CRIT_FAILURE,
>> + "sss_get_cased_name failed, skipping\n");
>> + continue;
>> + }
>> +
>> DEBUG(SSSDBG_TRACE_FUNC, "Adding group %s\n",
groupnames[i]);
>> }
Both suggestions make sense, please see the attached patch. Thank you
for the review.
Ack.