On 10/16/2014 01:37 PM, Sumit Bose wrote:
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

Hi,

+                        <term>ipa_user_override_object_class (string)</term>
+                        <listitem>
+                            <para>
+                                Name if the objectclass for user overrides. It
                                        ^ *of*
+                                is use to determine if the found override 
object
                                      ^ *used*
+                                related to a user or a group.
                                   ^ *is related*
+                            </para>

The same typos are also in description of ipa_group_override_object_class.

Besides those typos it's a full ACK.

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to