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,
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.
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.
> 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.
> if (ret == EOK) {
> if (strcmp(tmp_str, view_name) == 0) {
> /* view name already known, mothing to do */
^ *typo*
> 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.
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.
LS