[SSSD] [PATCH] GPO: Terminate request on error

Jakub Hrozek jhrozek at redhat.com
Wed Oct 22 13:52:41 UTC 2014


On Wed, Oct 22, 2014 at 03:11:54PM +0200, Lukas Slebodnik wrote:
> On (22/10/14 14:26), Jakub Hrozek wrote:
> >Hi,
> >
> >attached is a patch for an issue I found when working on the SELinux
> >child.
> 
> >From d2bb267d4d7bdb39666cf95d8b457c16a51ef88a Mon Sep 17 00:00:00 2001
> >From: Jakub Hrozek <jhrozek at redhat.com>
> >Date: Tue, 21 Oct 2014 16:18:02 +0200
> >Subject: [PATCH] GPO: Terminate request on error
> >
> >---
> > src/providers/ad/ad_gpo.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> >diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c
> >index c81bd49e0c840afda19772f0591f51c9a3e2d617..83edbe4fb5a7e617cab95c0330ceae31392b18b2 100644
> >--- a/src/providers/ad/ad_gpo.c
> >+++ b/src/providers/ad/ad_gpo.c
> >@@ -3898,11 +3898,13 @@ static void gpo_cse_done(struct tevent_req *subreq)
> >               "ad_gpo_parse_gpo_child_response failed: [%d][%s]\n",
> >               ret, strerror(ret));
> >         tevent_req_error(req, ret);
> >+        return;
> >     } else if (child_result != 0){
> >         DEBUG(SSSDBG_CRIT_FAILURE,
> >               "Error in gpo_child: [%d][%s]\n",
> >               child_result, strerror(child_result));
> >         tevent_req_error(req, child_result);
> >+        return;
> >     }
> Pavel has already acked this patch.
> I just want to start discussion about how to prevent such problems.
> 
> On some places we use to have similar construct in done/immediately section.
> src/providers/ad/ad_domain_info.c-    if (ret != EOK) {
> src/providers/ad/ad_domain_info.c:        tevent_req_error(req, ret);
> src/providers/ad/ad_domain_info.c-    } else {
> src/providers/ad/ad_domain_info.c-        tevent_req_done(req);
> src/providers/ad/ad_domain_info.c-    }
> src/providers/ad/ad_domain_info.c-    tevent_req_post(req, ev);
> src/providers/ad/ad_domain_info.c-    return req;
> 
> Maybe we should use it also on other places.

This is a pattern we use in _send(), yes. Do you suggest that we use it
also in other functions?

I'm not sure tevent_req_done is normally needed, but we could say that
we prefer 'goto fail' instead.

I was also thinking about a macro tevent_req_terminate() that would also
return, but I know that macros that change the flow of program are
considered evil by most developers..



More information about the sssd-devel mailing list