[SSSD] [PATCH] SYSDB: Return errno, not ldb error code

Lukas Slebodnik lslebodn at redhat.com
Mon Aug 18 06:37:18 UTC 2014


On (18/08/14 08:31), Jakub Hrozek wrote:
>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 :-)

>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

I think we don't need review from native speaker.

ACK

LS



More information about the sssd-devel mailing list