[Freeipa-devel] [PATCH] 0159-0160 Support ID views in compat tree

[Ludwig Krispenz]

On 10/07/2014 01:32 PM, Alexander Bokovoy wrote:
> On Tue, 07 Oct 2014, Alexander Bokovoy wrote:
>> On Tue, 07 Oct 2014, Ludwig Krispenz wrote:
>>>
>>> On 10/07/2014 12:39 PM, Alexander Bokovoy wrote:
>>>> On Tue, 07 Oct 2014, Ludwig Krispenz wrote:
>>>>>
>>>>> On 10/07/2014 12:12 PM, Alexander Bokovoy wrote:
>>>>>> On Tue, 07 Oct 2014, Ludwig Krispenz wrote:
>>>>>>> Hi Alex,
>>>>>>>
>>>>>>> slapi_attr_set_valueset() calls slapi_valueset_set_valueset(), 
>>>>>>> but this does not free the existing values,
>>>>>> Here is the problem, I cannot free original values as
>>>>>> slapi_attr_get_valueset() returns a copy and I never can get 
>>>>>> access to
>>>>>> the original Slapi_ValueSet.
>>>>> good point.
>>>>>>
>>>>>> I only can get access to the original Slapi_Value** array which 
>>>>>> means I
>>>>>> could manipulate its content but that probably require resizing 
>>>>>> it and
>>>>>> thus changing a pointer vs->va, to which I don't have access anyway.
>>>>> you should never do this, a valueset contains more data than va
>>>>>> How can we fix this?
>>>>> the best thing would probably be to fix slapi_attr_set_valueset or 
>>>>> adding a new api function which frees teh old valueset, but this 
>>>>> would add a dependency to specific DS version
>>>> What about something like this:
>>>>
>>>> void
>>>> slapi_valueset_set_valueset(Slapi_ValueSet *vs1, const Slapi_ValueSet
>>>> *vs2)
>>>> {
>>>>   if ((vs1 != NULL) && (vs1->num != 0)) {
>>>>       slapi_valueset_done(vs1);
>>>>   }
>>>>      slapi_valueset_init(vs1);
>>>>      valueset_set_valueset(vs1,vs2);
>>>> }
>>>>
>>>> Another option is to fix slapi_attr_set_valueset():
>>>>
>>>> int
>>>> slapi_attr_set_valueset(Slapi_Attr *a, const Slapi_ValueSet *vs)
>>>> {
>>>>   if (a->a_present_values->num != 0) {
>>>>       lapi_valueset_done(&a->a_present_values);
>>>>   }
>>>>      slapi_valueset_set_valueset( &a->a_present_values, vs);
>>>>  return 0;
>>>> }
>>>>
>>>> Other uses of slapi_valueset_set_valueset() are operating on newly
>>>> created Slapi_ValueSet instances.
>>> both will be ok (for me), but we have to commit it first and you 
>>> depend on that version.
>>>
>>> a more clumsy way would be to get the values and then use 
>>> slapi_entry_attr_replace_sv() which would free the attr and recreate it
>> Actually, you pointed me to a better solution:
>>
>> /* Replace the attribute's value with the override */
>>  result = slapi_entry_attr_exists(entry, override_type);
>>  if (result == 0) {
>>       result = slapi_attr_get_values(override_attr, &override_valueset);
>>       result = slapi_entry_attr_delete_sv(entry, override_type, NULL);
>>       result = slapi_entry_add_valueset(entry, override_type, 
>> override_valueset);
>>  }
>>
>> slapi_entry_delete_values_sv(entry, attr, NULL) will clear whole
>> attribute and then I can add the valueset. This, unfortunately, works
>> via iterating over the whole valueset and adding values one by one but
>> in our case it would be most likely a single element anyway.
> Pasted a lot of typos but fixed it already:
>  result = slapi_entry_attr_exists(entry, override_type);
>  if (result == 0) {
>       result = slapi_attr_get_valueset(override_attr, 
> &override_valueset);
>       result = slapi_entry_attr_delete(entry, override_type);
>       result = slapi_entry_add_valueset(entry, override_type, 
> override_valueset);
>  }
ok