[SSSD] [PATCH] SYSDB: Return errno, not ldb error code
Jakub Hrozek
jhrozek at redhat.com
Mon Aug 18 06:31:38 UTC 2014
On Mon, Aug 18, 2014 at 08:11:29AM +0200, Lukas Slebodnik wrote:
> On (17/08/14 20:45), Jakub Hrozek wrote:
> >Hi,
> >
> >for one of my patches I tried to use ldb permissive modification before
> >realising it's not the best approach. At least I found this little bug
> >in the sysdb code.
>
> >From de9f9376b6d5ed098a268a7a1f5d04bca1c8d96f Mon Sep 17 00:00:00 2001
> >From: Jakub Hrozek <jhrozek at redhat.com>
> >Date: Sat, 16 Aug 2014 19:41:40 +0200
> >Subject: [PATCH] SYSDB: Return errno, not ldb error code
> >
> >---
> > src/db/sysdb_ops.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> >diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
> >index 252097f28b7634d13d91392b63abdf0640bcfa19..f4d48e89c49879ea282c9e7637e8255028e95758 100644
> >--- a/src/db/sysdb_ops.c
> >+++ b/src/db/sysdb_ops.c
> >@@ -159,6 +159,9 @@ int sss_ldb_modify_permissive(struct ldb_context *ldb,
> >
> > talloc_free(req);
> >
> >+ if (ret) {
> >+ ret = sysdb_error_to_errno(ret);
> >+ }
> > return ret;
> > }
> NACK
>
> Current code expect LDB error codes to be returned from
> sss_ldb_modify_permissive. See src/db/sysdb_upgrade.c.
> But you can write doxygen comment for this function ;-)
D'oh. Thanks, I guess this is why we have code reviews.
OK, attached is a patch that adds comment to both the comment block
before the function and the return statement. I think adding the comment
is worth it because otherwise someone might spend an hour tracing why
does the code return ENOTDIR like I did before realizing it's actually
LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS :-)
-------------- next part --------------
>From a73b981e7b63a1d7a976bdc570184537de874769 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Sat, 16 Aug 2014 19:41:40 +0200
Subject: [PATCH] SYSDB: Clarify sss_ldb_modify_permissive returns ldb error
code
---
src/db/sysdb_ops.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 252097f28b7634d13d91392b63abdf0640bcfa19..e32d79a70c1c408f077b5a066acad9f200e36261 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -129,6 +129,9 @@ done:
* The wrapper around ldb_modify that uses LDB_CONTROL_PERMISSIVE_MODIFY_OID
* so that on adds entries that already exist are skipped and similarly
* entries that are missing are ignored on deletes
+ *
+ * Please note this function returns LDB error codes, not sysdb error
+ * codes on purpose, see usage in callers!
*/
int sss_ldb_modify_permissive(struct ldb_context *ldb,
struct ldb_message *msg)
@@ -159,6 +162,9 @@ int sss_ldb_modify_permissive(struct ldb_context *ldb,
talloc_free(req);
+ /* Please note this function returns LDB error codes, not sysdb error
+ * codes on purpose, see usage in callers!
+ */
return ret;
}
--
1.9.3
More information about the sssd-devel
mailing list