[SSSD] [sssd][patch] SYSDB: sysdb_getnetgr returns ENOENT

Jakub Hrozek jhrozek at redhat.com
Tue May 27 15:01:22 UTC 2014


On Tue, Apr 22, 2014 at 12:02:55PM +0200, Pavel Reichl wrote:
> On Tue, 2014-04-22 at 08:46 +0200, Lukas Slebodnik wrote:
> > On (17/04/14 18:44), Pavel Reichl wrote:
> > >Hello,
> > >
> > >attached patch is the first of many to solve
> > >https://fedorahosted.org/sssd/ticket/1991
> > >
> > >"The return codes of various sysdb operations differ. Some search
> > >operations would return ENOENT if they don't find a matching object some
> > >would return EOK but an empty result list."
> > >
> > >I think it would be best if in case that no results were found both
> > >ENOENT value and 'properly' empty list were returned.
> > >
> > >Thank for opinions or/and review.
> > >
> > >Pavel Reichl
> > 
> > yet another comment.
> > 
> > >From 1e7f69536acfb50e221807b59446ed9fb584bab9 Mon Sep 17 00:00:00 2001
> > >From: Pavel Reichl <preichl at redhat.com>
> > >Date: Wed, 26 Feb 2014 16:52:26 +0000
> > >Subject: [PATCH 1/5] SYSDB: sysdb_getnetgr returns ENOENT
> > >
> > >sysdb_getnetgr did not return ENOENT althought it was expected.
> > >
> > >Resolves:
> > >https://fedorahosted.org/sssd/ticket/1991
> > >---
> > > src/db/sysdb.h        | 1 +
> > > src/db/sysdb_search.c | 7 ++++++-
> > > 2 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > >diff --git a/src/db/sysdb.h b/src/db/sysdb.h
> > >index 5d337ffd8c0f7fbe6808d50538192369102caf17..45d486c3fa32e393ddb0bfe38426f9c2a3611955 100644
> > >--- a/src/db/sysdb.h
> > >+++ b/src/db/sysdb.h
> > >@@ -455,6 +455,7 @@ struct sysdb_netgroup_ctx {
> > >     } value;
> > > };
> > > 
> > >+/* If no group is found ENOENT is returned and result count is set to 0 */
> > > errno_t sysdb_getnetgr(TALLOC_CTX *mem_ctx,
> > >                        struct sss_domain_info *domain,
> > >                        const char *netgroup,
> > >diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
> > >index 60ad61368d92d12bca3833bc08b0673c581ab882..97192115b90b985cb0ff42516cdc02a44a33acc9 100644
> > >--- a/src/db/sysdb_search.c
> > >+++ b/src/db/sysdb_search.c
> > >@@ -827,7 +827,12 @@ errno_t sysdb_getnetgr(TALLOC_CTX *mem_ctx,
> > >     }
> > > 
> > >     *res = talloc_steal(mem_ctx, result);
> > >-    ret = EOK;
> > >+
> > >+    if((*res)->count == 0) {
> >          ^^^^^^
> >     I do not see a reason of using output argument here. You can use local
> >     variable result.
> OK, fixed.
> 
> > >+        ret =  ENOENT;
> > >+    } else {
> > >+        ret = EOK;
> > >+    }
> > > 
> > > done:
> > >     talloc_zfree(tmp_ctx);
> > >-- 
> > >1.8.4.2
> > >
> > 
> > BTW
> > I would expect more reusable solution for ticket #1991 or do you want to add
> > the same snippet to other function?
> 
> Well, actually I don't know yet. I think that there are too many
> different functions, some of them call directly ldap_search others call
> it indirectly. At first I would prefer to concentrate to fix
> implementation to expected behavior and mainly to don't break anything.
> 
> But of course if/when I will see patterns for reusable solution I will
> propose it.
> 
> > In the function sysdb_error_to_errno ldb error LDB_ERR_NO_SUCH_OBJECT
> > is transformed to the ENOENT. Could you explain why it did not work?
> I don't think there's any problem with function sysdb_error_to_errno, it
> converts LDB_SUCCESS to EOK perfectly.
> 
> > 
> > LS
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> 

I think the sss_ldb_search utility function (with a unit test) would be
a good idea. We talked with Sumit about releasing beta2 before I vanish
for vacation next week, so no need to hurry. I think it would be fine to
postpone this work even further.

> From a587d4c9013cbf7dd099a9d1c961b4157bea6e05 Mon Sep 17 00:00:00 2001
> From: Pavel Reichl <preichl at redhat.com>
> Date: Wed, 26 Feb 2014 16:52:26 +0000
> Subject: [PATCH 1/5] SYSDB: sysdb_getnetgr returns ENOENT
> 
> sysdb_getnetgr did not return ENOENT althought it was expected.
> 
> Resolves:
> https://fedorahosted.org/sssd/ticket/1991
> ---
>  src/db/sysdb.h        | 1 +
>  src/db/sysdb_search.c | 7 ++++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/db/sysdb.h b/src/db/sysdb.h
> index 5d337ffd8c0f7fbe6808d50538192369102caf17..45d486c3fa32e393ddb0bfe38426f9c2a3611955 100644
> --- a/src/db/sysdb.h
> +++ b/src/db/sysdb.h
> @@ -455,6 +455,7 @@ struct sysdb_netgroup_ctx {
>      } value;
>  };
>  
> +/* If no group is found ENOENT is returned and result count is set to 0 */
            ^^^^^
    If no netgroup is found

>  errno_t sysdb_getnetgr(TALLOC_CTX *mem_ctx,
>                         struct sss_domain_info *domain,
>                         const char *netgroup,
> diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
> index 60ad61368d92d12bca3833bc08b0673c581ab882..c911445c95a666bb399b85e41799226d0cb433fd 100644
> --- a/src/db/sysdb_search.c
> +++ b/src/db/sysdb_search.c
> @@ -827,7 +827,12 @@ errno_t sysdb_getnetgr(TALLOC_CTX *mem_ctx,
>      }
>  
>      *res = talloc_steal(mem_ctx, result);
> -    ret = EOK;
> +
> +    if(result->count == 0) {

Please put a space after the if

> +        ret = ENOENT;
> +    } else {
> +        ret = EOK;
> +    }
>  
>  done:
>      talloc_zfree(tmp_ctx);
> -- 
> 1.8.4.2
> 

It would be nice to have a unit test.  I'm going to push the patch with
test_sysdb_search_return_ENOENT, so maybe add it there.



More information about the sssd-devel mailing list