[SSSD] [PATCH] LDAP: Do not print verbose DEBUG messages from providers that don't set UUID
Lukas Slebodnik
lslebodn at redhat.com
Wed Jun 3 16:29:01 UTC 2015
On (03/06/15 18:21), Jakub Hrozek wrote:
>On Wed, Jun 03, 2015 at 06:13:04PM +0200, Lukas Slebodnik wrote:
>> On (03/06/15 17:58), Jakub Hrozek wrote:
>> >I think ENOENT in this case is better because the caller might (and
>> >does) special-case the value to handle "no UUID" case separately.
>>
>> >From 762d47583e799bb956564ead9418a64a8dd494e1 Mon Sep 17 00:00:00 2001
>> >From: Jakub Hrozek <jhrozek at redhat.com>
>> >Date: Wed, 3 Jun 2015 17:57:29 +0200
>> >Subject: [PATCH] LDAP: Do not print verbose DEBUG messages from providers that
>> ^^^^
>> changes are in sydb and not in LDAP
>> > don't set UUID
>> >
>> >https://fedorahosted.org/sssd/ticket/2666
>> >---
>> > src/db/sysdb_ops.c | 7 ++++++-
>> > 1 file changed, 6 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
>> >index 9afb0d7d74cdbc91570f4fe6485790d2a4eeb4be..d34583787f1fce9bd07e272ae7549b8bd4f0b7dd 100644
>> >--- a/src/db/sysdb_ops.c
>> >+++ b/src/db/sysdb_ops.c
>> >@@ -3795,7 +3795,12 @@ errno_t sysdb_handle_original_uuid(const char *orig_name,
>> > struct ldb_message_element *el;
>> > char guid_str_buf[GUID_STR_BUF_SIZE];
>> >
>> >- if (orig_name == NULL || src_attrs == NULL || src_name == NULL
>> >+ if (orig_name == NULL) {
>> >+ /* This provider doesn't handle UUIDs */
>> >+ return ENOENT;
>> >+ }
>> >+
>> >+ if (src_attrs == NULL || src_name == NULL
>> > || dest_attrs == NULL || dest_name == NULL) {
>> > return EINVAL;
>> > }
>>
>> sh$ ./test_sysdb_utils
>> [==========] Running 1 test(s).
>> [ RUN ] test_sysdb_handle_original_uuid
>> 0x2 != 0x16
>> ../sssd/src/tests/cmocka/test_sysdb_utils.c:48: error: Failure!
>>
>> [ FAILED ] test_sysdb_handle_original_uuid
>> [==========] 1 test(s) run.
>> [ PASSED ] 0 test(s).
>> [ FAILED ] 1 test(s), listed below:
>> [ FAILED ] test_sysdb_handle_original_uuid
>>
>> 1 FAILED TEST(S)
>>
>
>Let's try it again.
>From 9c69b6f3069ab5728de8f0956a4f7d1cc0afba3b Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Wed, 3 Jun 2015 17:57:29 +0200
>Subject: [PATCH] LDAP: Do not print verbose DEBUG messages from providers that
> don't set UUID
>
>https://fedorahosted.org/sssd/ticket/2666
>---
> src/db/sysdb_ops.c | 7 ++++++-
> src/tests/cmocka/test_sysdb_utils.c | 10 +++++++++-
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
>diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
>index 9afb0d7d74cdbc91570f4fe6485790d2a4eeb4be..d34583787f1fce9bd07e272ae7549b8bd4f0b7dd 100644
>--- a/src/db/sysdb_ops.c
>+++ b/src/db/sysdb_ops.c
>@@ -3795,7 +3795,12 @@ errno_t sysdb_handle_original_uuid(const char *orig_name,
> struct ldb_message_element *el;
> char guid_str_buf[GUID_STR_BUF_SIZE];
>
>- if (orig_name == NULL || src_attrs == NULL || src_name == NULL
>+ if (orig_name == NULL) {
>+ /* This provider doesn't handle UUIDs */
>+ return ENOENT;
>+ }
>+
>+ if (src_attrs == NULL || src_name == NULL
> || dest_attrs == NULL || dest_name == NULL) {
> return EINVAL;
> }
>diff --git a/src/tests/cmocka/test_sysdb_utils.c b/src/tests/cmocka/test_sysdb_utils.c
>index 1e9baa88cd82e7631a1de3db7f567f1f050ca67b..b791f14b7fdfdd4e55343fc1c03a7fd55d8ed101 100644
>--- a/src/tests/cmocka/test_sysdb_utils.c
>+++ b/src/tests/cmocka/test_sysdb_utils.c
>@@ -45,7 +45,7 @@ static void test_sysdb_handle_original_uuid(void **state)
> struct ldb_val guid_val = {bin_guid, 16};
>
> ret = sysdb_handle_original_uuid(NULL, NULL, NULL, NULL, NULL);
>- assert_int_equal(ret, EINVAL);
>+ assert_int_equal(ret, ENOENT);
>
> src_attrs = sysdb_new_attrs(NULL);
> assert_non_null(src_attrs);
>@@ -63,6 +63,14 @@ static void test_sysdb_handle_original_uuid(void **state)
> ret = sysdb_attrs_add_string(src_attrs, "UUID", IPA_UUID);
> assert_int_equal(ret, EOK);
>
>+ ret = sysdb_handle_original_uuid(NULL, src_attrs, "GUID",
>+ dest_attrs, "def");
>+ assert_int_equal(ret, ENOENT);
>+
>+ ret = sysdb_handle_original_uuid("objectGUID", NULL, "GUID",
>+ dest_attrs, "def");
>+ assert_int_equal(ret, EINVAL);
>+
> ret = sysdb_handle_original_uuid("objectGUID", src_attrs, "GUID",
> dest_attrs, "def");
> assert_int_equal(ret, EOK);
Patch works as expected.
I have a slightly related question (maybe for Sumit, because he wrote patch)
Why do we print message if ENOENT is returned?
ret = sysdb_handle_original_uuid(
opts->group_map[SDAP_AT_GROUP_UUID].def_name,
attrs,
opts->group_map[SDAP_AT_GROUP_UUID].sys_name,
group_attrs, SYSDB_UUID);
if (ret != EOK) {
DEBUG((ret == ENOENT) ? SSSDBG_TRACE_ALL : SSSDBG_MINOR_FAILURE,
"Failed to retrieve UUID [%d][%s].\n", ret, sss_strerror(ret));
}
Would it be better to have next version?
if (ret != EOK && ret != ENOENT) {
DEBUG(SSSDBG_MINOR_FAILURE,
"Failed to retrieve UUID [%d][%s].\n", ret, sss_strerror(ret));
}
because if the provider does not handle UUIDs
then we needn't print message at all.
LS
More information about the sssd-devel
mailing list