On (04/11/14 10:10), Pavel Reichl wrote:
Hello,
please simple attached patches.
Thanks.
From f4eedb6ba21bf0c483ba8b037695ef02f5e61ed8 Mon Sep 17 00:00:00
2001
From: Pavel Reichl <preichl(a)redhat.com>
Date: Tue, 4 Nov 2014 08:52:54 +0000
Subject: [PATCH 1/2] SYSDB: sysdb_get_bool() return ENOENT & unit tests
sysdb_get_bool() return ENOENT if no result is found.
Also unit test for sysdb_get_bool() & sysdb_set_bool() was added.
Resolves:
https://fedorahosted.org/sssd/ticket/1991
---
Our CI would not like this patch.
Running suite(s): sysdb
99%: Checks: 827, Failures: 1, Errors: 0
../sssd/src/tests/sysdb-tests.c:4953:F:SYSDB Tests:test_sysdb_has_enumerated:0: Error
[2][No such file or directory] checking enumeration
There should not be patch which breaks unit tests.
src/db/sysdb.c | 11 +++++++++--
src/tests/sysdb-tests.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/src/db/sysdb.c b/src/db/sysdb.c
index 1f02585e747dda6aadde772f76f30d3d69c4cfc0..53230dc4a3ac67b7faf0c68f792a828502b0ffc1
100644
--- a/src/db/sysdb.c
+++ b/src/db/sysdb.c
@@ -1508,6 +1508,7 @@ errno_t sysdb_get_bool(struct sysdb_ctx *sysdb,
errno_t ret;
int lret;
const char *attrs[2] = {attr_name, NULL};
+ struct ldb_message_element *el;
tmp_ctx = talloc_new(NULL);
if (tmp_ctx == NULL) {
@@ -1530,7 +1531,7 @@ errno_t sysdb_get_bool(struct sysdb_ctx *sysdb,
* to contain this attribute.
*/
*value = false;
- ret = EOK;
+ ret = ENOENT;
I agree with this change.
goto done;
} else if (res->count != 1) {
DEBUG(SSSDBG_CRIT_FAILURE,
@@ -1539,7 +1540,13 @@ errno_t sysdb_get_bool(struct sysdb_ctx *sysdb,
goto done;
}
- *value = ldb_msg_find_attr_as_bool(res->msgs[0], attr_name, false);
+ el = ldb_msg_find_element(res->msgs[0], attr_name);
+ if (el == NULL || el->num_values == 0) {
+ ret = ENOENT;
+ goto done;
+ } else {
+ *value = ldb_msg_find_attr_as_bool(res->msgs[0], attr_name, false);
+ }
But I'm not sure whether we need this one as well.
It makes sense to return ENOENT for users, groups or string.
In my opinion, default value "false" for boolean is good enough.
From 325c84d5f9d685ab9793b357dc4070d3b926ced2 Mon Sep 17 00:00:00
2001
From: Pavel Reichl <preichl(a)redhat.com>
Date: Tue, 4 Nov 2014 08:55:55 +0000
Subject: [PATCH 2/2] LDAP: fix ldap_setup_enumeration() handling ENOENT
This patch fixes ldap_setup_enumeration() to handle ENOENT returned by
sysdb_has_enumerated().
Resolves:
https://fedorahosted.org/sssd/ticket/1991
---
src/providers/ldap/ldap_id_enum.c | 6 +++++-
src/tests/sysdb-tests.c | 7 +++----
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/providers/ldap/ldap_id_enum.c b/src/providers/ldap/ldap_id_enum.c
index 9ffa3e5d9737048e2f4508cea4edc28d6f8cda48..13d2a62544b3956165ef9eb480fb5b813c890fd4
100644
--- a/src/providers/ldap/ldap_id_enum.c
+++ b/src/providers/ldap/ldap_id_enum.c
@@ -41,7 +41,11 @@ errno_t ldap_setup_enumeration(struct be_ctx *be_ctx,
struct ldap_enum_ctx *ectx;
ret = sysdb_has_enumerated(sdom->dom, &has_enumerated);
- if (ret != EOK) {
+ if (ret == ENOENT) {
+ /* default value */
+ has_enumerated = false;
+ ret = EOK;
+ } else if (ret != EOK) {
return ret;
}
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index 7f1ca8755d7066788cb4706b923c5282417b59aa..c8be7f968160372c4ca23fa68671455f018d2a3f
100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -4943,10 +4943,9 @@ START_TEST(test_sysdb_has_enumerated)
fail_if(ret != EOK, "Could not set up the test");
ret = sysdb_has_enumerated(test_ctx->domain, &enumerated);
- fail_if(ret != EOK, "Error [%d][%s] checking enumeration",
- ret, strerror(ret));
-
- fail_if(enumerated, "Enumeration should default to false");
+ fail_if(ret != ENOENT,
+ "Error [%d][%s] checking enumeration ENOENT is expected",
+ ret, strerror(ret));
ret = sysdb_set_enumerated(test_ctx->domain, true);
fail_if(ret != EOK, "Error [%d][%s] setting enumeration",
This test
should be probably part of the 1st patch
becuase it fix failed test.
LS