[SSSD] [PATCH] Return error code from ipa_subdom_store
Jakub Hrozek
jhrozek at redhat.com
Wed Mar 6 14:05:26 UTC 2013
On Tue, Mar 05, 2013 at 11:06:30AM +0100, Jakub Hrozek wrote:
> On Tue, Mar 05, 2013 at 09:57:36AM +0100, Sumit Bose wrote:
> > On Tue, Mar 05, 2013 at 12:20:12AM +0100, Jakub Hrozek wrote:
> > > I don't think there is a reason to ignore error codes while storing
> > > subdomains.
> >
> > I see it the other way round, there is no reason to return an error if
> > storing a subdomain fails, because we cannot do anything about it. If an
> > error is returned here, the operation which triggered to subdomain
> > lookup, e.g. getpwnam() with a fully qualified name, will fail. But the
> > needed subdomain for this request might already be stored by a previous
> > sysdb_subdomain_store() call, so if we return EOK the request might
> > finish successfully.
> >
> > For this reasons I prefer to ignore the error. But if you think it would
> > be better that the original request fails so that the error is not
> > unnoticed I wouldn't be against it.
> >
> > bye,
> > Sumit
>
> Hm, you're probably right. I would still prefer to at least report an
> error with a DEBUG message and then return EOK or explicitly ignore the
> return code in the caller. But I think the code has to explicitly say
> that the error is being ignored on purpose, otherwise it's confusing.
>
> Thank you for the review, I will prepare another patch.
Attached is a patch that returns errno but only warns in the callers.
-------------- next part --------------
>From 778aadadc6f1d7d8dfb353032f24f968bbbee60c Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 5 Mar 2013 00:15:35 +0100
Subject: [PATCH] Return error code from ipa_subdom_store
---
src/providers/ipa/ipa_subdomains.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
index 7d6e5958ed1e8ff9ea663c9e3bc754442a46f3c6..3a2fac70eff3d2a05be96746b058233810177f39 100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -230,11 +230,13 @@ static errno_t ipa_subdom_store(struct sss_domain_info *domain,
ret = sysdb_subdomain_store(domain->sysdb, name, realm, flat, id);
if (ret) {
DEBUG(SSSDBG_OP_FAILURE, ("sysdb_subdomain_store failed.\n"));
+ goto done;
}
+ ret = EOK;
done:
talloc_free(tmp_ctx);
- return EOK;
+ return ret;
}
static errno_t
@@ -431,8 +433,11 @@ static errno_t ipa_subdomains_refresh(struct ipa_subdomains_ctx *ctx,
/* ok let's try to update it */
ret = ipa_subdom_store(domain, reply[c]);
if (ret) {
- DEBUG(SSSDBG_OP_FAILURE, ("Failed to parse subdom data\n"));
- goto done;
+ /* Nothing we can do about the erorr. Let's at least try
+ * to reuse the existing domain
+ */
+ DEBUG(SSSDBG_MINOR_FAILURE, ("Failed to parse subdom data, "
+ "will try to use cached subdomain\n"));
}
handled[c] = true;
h++;
@@ -452,10 +457,13 @@ static errno_t ipa_subdomains_refresh(struct ipa_subdomains_ctx *ctx,
if (handled[c]) {
continue;
}
+ /* Nothing we can do about the erorr. Let's at least try
+ * to reuse the existing domain.
+ */
ret = ipa_subdom_store(domain, reply[c]);
if (ret) {
- DEBUG(SSSDBG_OP_FAILURE, ("Failed to parse subdom data\n"));
- goto done;
+ DEBUG(SSSDBG_MINOR_FAILURE, ("Failed to parse subdom data, "
+ "will try to use cached subdomain\n"));
}
}
--
1.8.1.4
More information about the sssd-devel
mailing list