6:37 a.m.
On Wed, Oct 15, 2014 at 03:57:44PM +0200, Lukas Slebodnik wrote:
On (15/10/14 15:37), Pavel Březina wrote:
>On 10/14/2014 05:12 PM, Sumit Bose wrote:
>>On Tue, Oct 14, 2014 at 11:24:04AM +0200, Sumit Bose wrote:
>>>On Tue, Oct 14, 2014 at 11:16:28AM +0200, Sumit Bose wrote:
>>>>On Wed, Oct 01, 2014 at 08:14:21PM +0200, Sumit Bose wrote:
>>>>>On Mon, Sep 29, 2014 at 07:38:54PM +0200, Jakub Hrozek wrote:
>>>>>>On Mon, Sep 29, 2014 at 03:00:44PM +0200, Sumit Bose wrote:
>>>>>>>>Hi,
>>>>>>>>I'm sending two more trivial debug patches to squash
in. Otherwise its an
>>>>>>>>code wise ack. I'm running some tests now.
>>>>>>>>
>>>>>>>
>>>>>>>Thank you. Jakub, in case there are no other issues, can you
add them
>>>>>>>when committing the patches, or shall I send a new round?
>>>>>>>
>>>>>>>bye,
>>>>>>>Sumit
>>>>>>
>>>>>>I will run the patches through the IPA CI engine with Tomas'
help. If
>>>>>>there are no regressions, I will squash in the patches and push,
if
>>>>>>there are, we would do another round.
>>>>>
>>>>>Hi,
>>>>>
>>>>>this new series of patches contain fixes for various issues found by
>>>>>Pavel, Jakub and Alexander. There is an additional 9th patch which
>>>>>contains support for searching in the override values, e.g. if the
user
>>>>>name is overridden we have to first search in the override data and
the
>>>>>continue with the original object. See commit message for details.
>>>>
>>>>Please find attached the latest version of the patches with even more
>>>>fixes for issue found by Alexander, Jakub and Pavel. I tired to keep the
>>>>original structure of the patches but a new patch sneaked in which adds
>>>>some new sysdb interface which allow to add attribute values with
>>>>detection of duplicates.
>>>>
>>>>bye,
>>>>Sumit
>>>
>>>I forgot the say that the patches now depend on the two patches I send
>>>in the "[PATCHES] nss: add SSS_NSS_GETORIGBYNAME request" thread.
>>
>>This new version fixes some minor issues found by Coverity.
>
>Hi,
Pavel, Lukas, thank you for the review.
>
>Patch 1: sysdb: add sysdb_update_view_name()
>
>>static errno_t sysdb_get_view_name_ex(TALLOC_CTX *mem_ctx,
>> struct sysdb_ctx *sysdb,
>> char **_view_name,
>> bool *view_container_exists)
>>{
>...
>>
>> *_view_name = talloc_strdup(mem_ctx, tmp_str);
>
>I think it is possible to just steal the value since ldb uses talloc. But if
>you want to strdup the string you should test for NULL.
good point, I steal it.
>
>Do we really need view_container_exists? I think it is not possible to have a
>view without a name and I would rather report an error in this case since it
>basically means that db is corrupted.
I would prefer to keep it. You are right, currently the code always
creates the container with a name but I would like to be on the save
side and avoid issues when creating ing the container to a second time.
>
>> ret = EOK;
>>
>>done:
>> talloc_free(tmp_ctx);
>> return ret;
>>}
>
>>errno_t sysdb_update_view_name(struct sysdb_ctx *sysdb,
>> const char *view_name)
>>{
>> errno_t ret;
>> TALLOC_CTX *tmp_ctx;
>> char *tmp_str;
>> bool view_container_exists = false;
>> bool add_view_name = false;
>> struct ldb_message *msg;
>
>I suppose this function is not yet completed? add_view_name is basically not
>used since it is always true.
yes, it will be used when we allow to change the view.
>
>> if (ret == EOK) {
>> if (strcmp(tmp_str, view_name) == 0) {
>> /* view name already known, mothing to do */
> ^ *typo*
fixed
>
>> DEBUG(SSSDBG_TRACE_ALL, "View name already in place.\n");
>> ret = EOK;
>> goto done;
>
>Patch 2: Add sdap_deref_search_with_filter_send()
>
>ACK
>
>Patch 3: IPA: make IPA ID context available to extdom client code
>
>ACK
>
>Patch 4: IPA: add view support and get view name
>
>ACK
>
>Do you think the list of options is now complete? The new options are still
>missing in man page and config api.
added
>
>Patch 5: views: add ipa_get_ad_override_send()
>
>ACK
>
>Patch 6: sysdb: add sysdb_store_override
>
>ACK
>
>Patch 7: sysdb: sysdb_apply_default_override
>
Smal nitpick from our CI, after applying 7th path there is an error
src/db/sysdb_views.c: In function 'safe_original_attributes':
src/db/sysdb_views.c:539:13: error: implicit declaration of function
'sysdb_attrs_add_val_safe' [-Werror=implicit-function-declaration]
ret = sysdb_attrs_add_val_safe(attrs, SYSDB_NAME_ALIAS,
^
cc1: all warnings being treated as errors
The solution is simple: just change order of patches.
0008-sysdb-add-sysdb_attrs_add_val_safe-and-sysdb_attrs_a.patch
0007-sysdb-sysdb_apply_default_override.patch
The function sysdb_attrs_add_val_safe was defined in 8th patch,
but was already used in 7th patch.
The order is switched now.
New version attached.
bye,
Sumit
LS
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel