On Fri, Feb 26, 2016 at 11:08:45AM +0100, Pavel Březina wrote:
> On 02/24/2016 03:19 PM, Jakub Hrozek wrote:
>> Hi,
>>
>> the attached patch fixes:
>>
https://fedorahosted.org/sssd/ticket/2959
>>
>> It was confirmed by the original reporter. The bug was there since 2009,
>> by the way, I'm really suprised we only caught it now..
>
> Good job finding this. I'm inclined to ack this patch, I have one question
> though:
>
>> if (is_user && diff[0]) {
>> /* file memberuid removal operations */
>> name = ldb_msg_find_attr_as_string(delop->entry, DB_NAME, NULL);
>> if (!name) {
>> return LDB_ERR_OPERATIONS_ERROR;
>> }
>>
>> for (i = 0; diff[i]; i++) {
>> ret = mbof_append_muop(del_ctx, &del_ctx->muops,
>> &del_ctx->num_muops,
>> LDB_FLAG_MOD_DELETE,
>> diff[i], name,
>> DB_MEMBERUID);
>
> This invokes assignment: del_ctx->muops[j] = diff[i]. To be thorough we
> should talloc_steal(del_ctx->muops, diff[i]) here to ensure proper talloc
> hierarchy. But it is not necessary since delctx exists only for one
> operation... what do you think?
Yes, I think it makes sense.