On (04/12/15 16:42), Jakub Hrozek wrote:
On Thu, Dec 03, 2015 at 01:24:54PM +0100, Pavel Březina wrote:
> On 11/20/2015 12:04 PM, Jakub Hrozek wrote:
> >On Thu, Nov 19, 2015 at 01:51:54PM +0100, Pavel Březina wrote:
> >>>Thanks, new patches are attached.
> >>
> >>I had a phone call with Jakub and we decided that it will be better to use
> >>be_req directly instead of lower level sbus_req. This will allow us to
> >>simplify it more.
> >
> >OK, see the attached patches..
>
> Hi, thanks for the changes.
>
> First patch:
>
> >@@ -199,6 +205,7 @@ struct be_req *be_req_create(TALLOC_CTX *mem_ctx,
> > be_req->domain = be_ctx->domain;
> > be_req->fn = fn;
> > be_req->pvt = pvt_fn_data;
> >+ be_req->req_name = name;
>
> I know every call now passes static data but IMHO we should be safe and use
> talloc_strdup here.
>
> >+static errno_t be_sbus_reply(struct sbus_request *sbus_req,
> >+ dbus_uint16_t err_maj,
> >+ dbus_uint32_t err_min,
> >+ const char *err_msg)
> >+{
> >+ errno_t ret;
> >+ const char *safe_err_msg;
> >+
> >+ /* Only return a reply if one was requested
> >+ * There may not be one if this request began
> >+ * while we were offline
> >+ */
> >+ if (sbus_req == NULL) {
> >+ return EOK;
> >+ }
> >+
> >+ safe_err_msg = safe_be_req_err_msg(err_msg, err_maj);
> >+
> >+ if (err_maj == DP_ERR_FATAL && err_min == ENODEV) {
> >+ DEBUG(SSSDBG_TRACE_LIBS, "Handler not configured\n");
> >+ } else {
> >+ DEBUG(SSSDBG_TRACE_LIBS,
> >+ "Request processed. Returned %d,%d,%s\n",
> >+ err_maj, err_min, err_msg);
> >+ }
>
> Can we translate err_maj into string here? To get message similar to:
> "Request processed: Success [%d]: %s, err_min, err_msg
> "Request processed: Failure [%d]: %s, err_min, err_msg
> "Request processed: Offline [%d]: %s, err_min, err_msg
>
> Second patch:
>
> >+#define BE_SBUS_REPLY_DATA_INIT { .err_maj = DP_ERR_FATAL, \
> >+ .err_min = EFAULT, \
> >+ .err_msg = "Fatal error" \
> >+ };
>
> ERR_INTERNAL might be a better choice over EFAULT since it basically means
> we forgot to set it -> code error.
OK, see the attached two.
From 0bff8916f35a39ff8fdd4789a31289f7d1b05877 Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Wed, 11 Nov 2015 13:39:43 +0100
Subject: [PATCH 1/2] DP: Reduce code duplication in the callback handlers
Instead of calling sbus_request_return_and_finish() directly with the
same checks copied over, add a be_sbus_reply() helper instead.
---
src/providers/ad/ad_subdomains.c | 2 +-
src/providers/data_provider_be.c | 355 +++++++++++++------------------------
src/providers/dp_backend.h | 11 +-
src/providers/ipa/ipa_subdomains.c | 2 +-
4 files changed, 138 insertions(+), 232 deletions(-)
diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
index 2e5d9120e473e32a84610d607ccf329249b4ac9e..4799b518a1354e5b4ef8392b860effc9121ee121
100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -1118,7 +1118,7 @@ static void ad_subdom_online_cb(void *pvt)
refresh_interval = ctx->be_ctx->domain->subdomain_refresh_interval;
- be_req = be_req_create(ctx, NULL, ctx->be_ctx,
+ be_req = be_req_create(ctx, NULL, ctx->be_ctx, "AD subdomains",
ad_subdom_be_req_callback, NULL);
if (be_req == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "be_req_create() failed.\n");
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index 9bc54b6d59dafd22b9f70da7cca3b520ca941efc..b67efa87008d8e15fb9fb27c3d02996608b0aa62
100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -174,6 +174,9 @@ struct be_req {
*/
int phase;
+ /* Just for nicer debugging */
+ const char *req_name;
+
struct be_req *prev;
struct be_req *next;
};
@@ -186,8 +189,11 @@ static int be_req_destructor(struct be_req *be_req)
}
struct be_req *be_req_create(TALLOC_CTX *mem_ctx,
- struct be_client *becli, struct be_ctx *be_ctx,
- be_async_callback_t fn, void *pvt_fn_data)
+ struct be_client *becli,
+ struct be_ctx *be_ctx,
+ const char *name,
+ be_async_callback_t fn,
+ void *pvt_fn_data)
{
struct be_req *be_req;
@@ -199,6 +205,11 @@ struct be_req *be_req_create(TALLOC_CTX *mem_ctx,
be_req->domain = be_ctx->domain;
be_req->fn = fn;
be_req->pvt = pvt_fn_data;
+ be_req->req_name = talloc_strdup(be_req, name);
+ if (be_req->req_name == NULL) {
+ talloc_free(be_req);
+ return NULL;
+ }
/* Add this request to active request list and make sure it is
* removed on termination. */
@@ -242,6 +253,95 @@ void be_req_terminate(struct be_req *be_req,
be_req->fn(be_req, dp_err_type, errnum, errstr);
}
+
I know it's a nitpick, but you added extra new line here :-)
and you should change 2nd patch anyway.
+static errno_t be_sbus_reply(struct sbus_request *sbus_req,
+ dbus_uint16_t err_maj,
+ dbus_uint32_t err_min,
+ const char *err_msg)
+{
+ errno_t ret;
+ const char *safe_err_msg;
+
+ /* Only return a reply if one was requested
+ * There may not be one if this request began
+ * while we were offline
+ */
+ if (sbus_req == NULL) {
+ return EOK;
+ }
+
+ safe_err_msg = safe_be_req_err_msg(err_msg, err_maj);
+
+ if (err_maj == DP_ERR_FATAL && err_min == ENODEV) {
+ DEBUG(SSSDBG_TRACE_LIBS, "Handler not configured\n");
+ } else {
+ DEBUG(SSSDBG_TRACE_LIBS,
+ "Request processed. Returned [%s]:%d,%d,%s\n",
+ dp_err_to_string(err_maj), err_maj, err_min, err_msg);
+ }
+
+ ret = sbus_request_return_and_finish(sbus_req,
+ DBUS_TYPE_UINT16, &err_maj,
+ DBUS_TYPE_UINT32, &err_min,
+ DBUS_TYPE_STRING, &safe_err_msg,
+ DBUS_TYPE_INVALID);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "sbus_request_return_and_finish failed: [%d]: %s\n",
+ ret, sss_strerror(ret));
+ }
+
+ return ret;
+}
+
+static errno_t be_sbus_req_reply(struct sbus_request *sbus_req,
+ int dp_err_type,
+ int errnum,
+ const char *errstr)
+{
+ dbus_uint16_t err_maj;
+ dbus_uint32_t err_min;
+
+ err_maj = dp_err_type;
+ err_min = errnum;
+
+ return be_sbus_reply(sbus_req, err_maj, err_min, errstr);
+}
+
+static void be_req_default_callback(struct be_req *be_req,
+ int dp_err_type,
+ int errnum,
+ const char *errstr)
+{
+ struct sbus_request *dbus_req;
+
+ DEBUG(SSSDBG_TRACE_FUNC, "Replying to %s request\n", be_req->req_name);
+
+ dbus_req = (struct sbus_request *) be_req->pvt;
+
+ be_sbus_req_reply(dbus_req, dp_err_type, errnum, errstr);
+ talloc_free(be_req);
+}
+
+/* Send back an immediate reply and set the sbus_request to NULL
+ * so that we are sure the request is not reused in the future
+ */
+static errno_t be_offline_reply(struct sbus_request **sbus_req_ptr)
+{
+ struct sbus_request *dbus_req;
+ errno_t ret;
+
+ if (sbus_req_ptr == NULL) {
+ return EINVAL;
+ }
+ dbus_req = *sbus_req_ptr;
+
+ ret = be_sbus_req_reply(dbus_req, DP_ERR_OFFLINE, EAGAIN,
+ "Fast reply - offline");
+ *sbus_req_ptr = NULL;
+ return ret;
+}
+
void be_terminate_domain_requests(struct be_ctx *be_ctx,
const char *domain)
{
@@ -439,9 +539,6 @@ static void be_queue_next_request(struct be_req *be_req, enum bet_type
type)
struct bet_queue_item **req_queue;
struct sbus_request *dbus_req;
int ret;
- uint16_t err_maj;
- uint32_t err_min;
- const char *err_msg = "Cannot file back end request";
struct be_req *next_be_req = NULL;
req_queue = &be_req->becli->bectx->bet_info[type].req_queue;
@@ -481,21 +578,8 @@ static void be_queue_next_request(struct be_req *be_req, enum
bet_type type)
dbus_req = (struct sbus_request *) next_be_req->pvt;
- if (dbus_req) {
- /* Return a reply if one was requested
- * There may not be one if this request began
- * while we were offline
- */
- err_maj = DP_ERR_FATAL;
- err_min = ret;
-
- sbus_request_return_and_finish(dbus_req,
- DBUS_TYPE_UINT16, &err_maj,
- DBUS_TYPE_UINT32, &err_min,
- DBUS_TYPE_STRING, &err_msg,
- DBUS_TYPE_INVALID);
- }
-
+ be_sbus_req_reply(dbus_req, DP_ERR_FATAL, ret,
+ "Cannot file back end request");
talloc_free(next_be_req);
}
@@ -663,34 +747,12 @@ static void get_subdomains_callback(struct be_req *req,
const char *errstr)
{
struct sbus_request *dbus_req;
- dbus_uint16_t err_maj = 0;
- dbus_uint32_t err_min = 0;
- const char *err_msg = NULL;
-
- DEBUG(SSSDBG_TRACE_FUNC, "Backend returned: (%d, %d, %s) [%s]\n",
- dp_err_type, errnum, errstr?errstr:"<NULL>",
- dp_err_to_string(dp_err_type));
be_queue_next_request(req, BET_SUBDOMAINS);
- dbus_req = (struct sbus_request *)req->pvt;
-
- if (dbus_req) {
- /* Return a reply if one was requested
- * There may not be one if this request began
- * while we were offline
- */
- err_maj = dp_err_type;
- err_min = errnum;
- err_msg = safe_be_req_err_msg(errstr, dp_err_type);
-
- sbus_request_return_and_finish(dbus_req,
- DBUS_TYPE_UINT16, &err_maj,
- DBUS_TYPE_UINT32, &err_min,
- DBUS_TYPE_STRING, &err_msg,
- DBUS_TYPE_INVALID);
- }
+ dbus_req = (struct sbus_request *) req->pvt;
+ be_sbus_req_reply(dbus_req, dp_err_type, errnum, errstr);
talloc_free(req);
}
@@ -737,7 +799,7 @@ static int be_get_subdomains(struct sbus_request *dbus_req, void
*user_data)
/* process request */
- be_req = be_req_create(becli, becli, becli->bectx,
+ be_req = be_req_create(becli, becli, becli->bectx, "get subdomains",
get_subdomains_callback, dbus_req);
if (!be_req) {
err_maj = DP_ERR_FATAL;
@@ -797,42 +859,6 @@ immediate:
return EOK;
}
-static void acctinfo_callback(struct be_req *req,
- int dp_err_type,
- int errnum,
- const char *errstr)
-{
- struct sbus_request *dbus_req;
- dbus_uint16_t err_maj = 0;
- dbus_uint32_t err_min = 0;
- const char *err_msg = NULL;
-
- dbus_req = (struct sbus_request *)req->pvt;
-
- if (dbus_req) {
- /* Return a reply if one was requested
- * There may not be one if this request began
- * while we were offline
- */
-
- err_maj = dp_err_type;
- err_min = errnum;
- err_msg = safe_be_req_err_msg(errstr, dp_err_type);
-
- sbus_request_return_and_finish(dbus_req,
- DBUS_TYPE_UINT16, &err_maj,
- DBUS_TYPE_UINT32, &err_min,
- DBUS_TYPE_STRING, &err_msg,
- DBUS_TYPE_INVALID);
-
- DEBUG(SSSDBG_CONF_SETTINGS, "Request processed. Returned %d,%d,%s\n",
- err_maj, err_min, err_msg);
- }
-
- /* finally free the request */
- talloc_free(req);
-}
-
struct be_initgr_prereq {
char *user;
char *domain;
@@ -851,8 +877,8 @@ static void acctinfo_callback_initgr_wrap(struct be_req *be_req)
struct be_initgr_prereq);
be_req->pvt = pr->orig_pvt_data;
- acctinfo_callback(be_req, pr->orig_dp_err_type,
- pr->orig_errnum, pr->orig_errstr);
+ be_req_default_callback(be_req, pr->orig_dp_err_type,
+ pr->orig_errnum, pr->orig_errstr);
}
static void acctinfo_callback_initgr_sbus(DBusPendingCall *pending, void *ptr)
@@ -1072,7 +1098,7 @@ be_get_account_info_send(TALLOC_CTX *mem_ctx,
struct be_get_account_info_state);
if (!req) return NULL;
- be_req = be_req_create(state, becli, be_ctx,
+ be_req = be_req_create(state, becli, be_ctx, "get account info",
be_get_account_info_done, req);
if (!be_req) {
ret = ENOMEM;
@@ -1187,24 +1213,11 @@ static int be_get_account_info(struct sbus_request *dbus_req, void
*user_data)
* return offline immediately
*/
if ((type & BE_REQ_FAST) && becli->bectx->offstat.offline) {
- /* Send back an immediate reply */
- err_maj = DP_ERR_OFFLINE;
- err_min = EAGAIN;
- err_msg = "Fast reply - offline";
-
- ret = sbus_request_return_and_finish(dbus_req,
- DBUS_TYPE_UINT16, &err_maj,
- DBUS_TYPE_UINT32, &err_min,
- DBUS_TYPE_STRING, &err_msg,
- DBUS_TYPE_INVALID);
+ ret = be_offline_reply(&dbus_req);
if (ret != EOK) {
return ret;
}
- DEBUG(SSSDBG_CONF_SETTINGS, "Request processed. Returned %d,%d,%s\n",
- err_maj, err_min, err_msg);
-
- dbus_req = NULL;
/* This reply will be queued and sent
* when we reenter the mainloop.
*
@@ -1213,8 +1226,8 @@ static int be_get_account_info(struct sbus_request *dbus_req, void
*user_data)
*/
}
- be_req = be_req_create(becli, becli, becli->bectx,
- acctinfo_callback, dbus_req);
+ be_req = be_req_create(becli, becli, becli->bectx, "get account info",
+ be_req_default_callback, dbus_req);
if (!be_req) {
err_maj = DP_ERR_FATAL;
err_min = ENOMEM;
@@ -1424,7 +1437,7 @@ static int be_pam_handler(struct sbus_request *dbus_req, void
*user_data)
becli = talloc_get_type(user_data, struct be_client);
if (!becli) return EINVAL;
- be_req = be_req_create(becli, becli, becli->bectx,
+ be_req = be_req_create(becli, becli, becli->bectx, "PAM",
be_pam_handler_callback, dbus_req);
if (!be_req) {
DEBUG(SSSDBG_TRACE_LIBS, "talloc_zero failed.\n");
@@ -1536,44 +1549,6 @@ done:
return EOK;
}
-static void be_sudo_handler_reply(struct sbus_request *dbus_req,
- dbus_uint16_t dp_err,
- dbus_uint32_t dp_ret,
- const char *errstr)
-{
- const char *err_msg = NULL;
-
- if (dbus_req == NULL) {
- return;
- }
-
- err_msg = errstr ? errstr : "No errmsg set\n";
- sbus_request_return_and_finish(dbus_req,
- DBUS_TYPE_UINT16, &dp_err,
- DBUS_TYPE_UINT32, &dp_ret,
- DBUS_TYPE_STRING, &err_msg,
- DBUS_TYPE_INVALID);
-
- DEBUG(SSSDBG_FUNC_DATA, "SUDO Backend returned: (%d, %d, %s)\n",
- dp_err, dp_ret, errstr ? errstr :
"<NULL>");
-}
-
-static void be_sudo_handler_callback(struct be_req *req,
- int dp_err,
- int dp_ret,
- const char *errstr)
-{
- const char *err_msg = NULL;
- struct sbus_request *dbus_req;
-
- dbus_req = (struct sbus_request *)(req->pvt);
-
- err_msg = safe_be_req_err_msg(errstr, dp_err);
- be_sudo_handler_reply(dbus_req, dp_err, dp_ret, err_msg);
-
- talloc_free(req);
-}
-
static int be_sudo_handler(struct sbus_request *dbus_req, void *user_data)
{
DBusError dbus_error;
@@ -1597,8 +1572,8 @@ static int be_sudo_handler(struct sbus_request *dbus_req, void
*user_data)
}
/* create be request */
- be_req = be_req_create(be_cli, be_cli, be_cli->bectx,
- be_sudo_handler_callback, dbus_req);
+ be_req = be_req_create(be_cli, be_cli, be_cli->bectx, "sudo",
+ be_req_default_callback, dbus_req);
if (be_req == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zero failed.\n");
return ENOMEM;
@@ -1621,9 +1596,8 @@ static int be_sudo_handler(struct sbus_request *dbus_req, void
*user_data)
* return offline immediately
*/
if ((type & BE_REQ_FAST) && be_cli->bectx->offstat.offline) {
- be_sudo_handler_reply(dbus_req, DP_ERR_OFFLINE, EAGAIN,
- "Fast reply - offline");
- be_req->pvt = dbus_req = NULL;
+ be_offline_reply(&dbus_req);
+ be_req->pvt = NULL;
/* This reply will be queued and sent
* when we reenter the mainloop.
*
@@ -1732,16 +1706,11 @@ static int be_sudo_handler(struct sbus_request *dbus_req, void
*user_data)
fail:
/* send reply back immediately */
- be_sudo_handler_callback(be_req, DP_ERR_FATAL, ret,
- err_msg ? err_msg : strerror(ret));
+ be_req_default_callback(be_req, DP_ERR_FATAL, ret,
+ err_msg ? err_msg : strerror(ret));
return EOK;
}
-static void be_autofs_handler_callback(struct be_req *req,
- int dp_err_type,
- int errnum,
- const char *errstr);
-
static int be_autofs_handler(struct sbus_request *dbus_req, void *user_data)
{
struct be_client *be_cli = NULL;
@@ -1773,24 +1742,7 @@ static int be_autofs_handler(struct sbus_request *dbus_req, void
*user_data)
* return offline immediately
*/
if ((type & BE_REQ_FAST) && be_cli->bectx->offstat.offline) {
- /* Send back an immediate reply */
- err_maj = DP_ERR_OFFLINE;
- err_min = EAGAIN;
- err_msg = "Fast reply - offline";
-
- ret = sbus_request_return_and_finish(dbus_req,
- DBUS_TYPE_UINT16, &err_maj,
- DBUS_TYPE_UINT32, &err_min,
- DBUS_TYPE_STRING, &err_msg,
- DBUS_TYPE_INVALID);
- if (ret != EOK) {
- return ret;
- }
-
- DEBUG(SSSDBG_TRACE_LIBS, "Request processed. Returned %d,%d,%s\n",
- err_maj, err_min, err_msg);
-
- dbus_req = NULL;
+ be_offline_reply(&dbus_req);
/* This reply will be queued and sent
* when we reenter the mainloop.
*
@@ -1816,8 +1768,8 @@ static int be_autofs_handler(struct sbus_request *dbus_req, void
*user_data)
}
/* create be request */
- be_req = be_req_create(be_cli, be_cli, be_cli->bectx,
- be_autofs_handler_callback, dbus_req);
+ be_req = be_req_create(be_cli, be_cli, be_cli->bectx, "autofs",
+ be_req_default_callback, dbus_req);
if (be_req == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zero failed.\n");
err_maj = DP_ERR_FATAL;
@@ -1890,43 +1842,6 @@ done:
return EOK;
}
-static void be_autofs_handler_callback(struct be_req *req,
- int dp_err_type,
- int errnum,
- const char *errstr)
-{
- struct sbus_request *dbus_req;
- dbus_uint16_t err_maj = 0;
- dbus_uint32_t err_min = 0;
- const char *err_msg = NULL;
-
- dbus_req = (struct sbus_request *)req->pvt;
-
- if (dbus_req) {
- /* Return a reply if one was requested
- * There may not be one if this request began
- * while we were offline
- */
-
- err_maj = dp_err_type;
- err_min = errnum;
- err_msg = safe_be_req_err_msg(errstr, dp_err_type);
-
- sbus_request_return_and_finish(dbus_req,
- DBUS_TYPE_UINT16, &err_maj,
- DBUS_TYPE_UINT32, &err_min,
- DBUS_TYPE_STRING, &err_msg,
- DBUS_TYPE_INVALID);
-
- DEBUG(SSSDBG_TRACE_LIBS,
- "Request processed. Returned %d,%d,%s\n",
- err_maj, err_min, err_msg);
- }
-
- /* finally free the request */
- talloc_free(req);
-}
-
static int be_host_handler(struct sbus_request *dbus_req, void *user_data)
{
struct be_host_req *req;
@@ -1958,24 +1873,8 @@ static int be_host_handler(struct sbus_request *dbus_req, void
*user_data)
*/
if ((flags & BE_REQ_FAST) && becli->bectx->offstat.offline) {
/* Send back an immediate reply */
- err_maj = DP_ERR_OFFLINE;
- err_min = EAGAIN;
- err_msg = "Fast reply - offline";
+ be_offline_reply(&dbus_req);
- ret = sbus_request_return_and_finish(dbus_req,
- DBUS_TYPE_UINT16, &err_maj,
- DBUS_TYPE_UINT32, &err_min,
- DBUS_TYPE_STRING, &err_msg,
- DBUS_TYPE_INVALID);
- if (ret != EOK) {
- return ret;
- }
-
- DEBUG(SSSDBG_TRACE_LIBS,
- "Request processed. Returned %d,%d,%s\n",
- err_maj, err_min, err_msg);
-
- dbus_req = NULL;
/* This reply will be queued and sent
* when we reenter the mainloop.
*
@@ -1984,8 +1883,8 @@ static int be_host_handler(struct sbus_request *dbus_req, void
*user_data)
*/
}
- be_req = be_req_create(becli, becli, becli->bectx,
- acctinfo_callback, dbus_req);
+ be_req = be_req_create(becli, becli, becli->bectx, "hostinfo",
+ be_req_default_callback, dbus_req);
if (!be_req) {
err_maj = DP_ERR_FATAL;
err_min = ENOMEM;
@@ -2257,7 +2156,7 @@ static void check_if_online(struct be_ctx *ctx)
goto failed;
}
- be_req = be_req_create(ctx, NULL, ctx,
+ be_req = be_req_create(ctx, NULL, ctx, "online check",
check_online_callback, NULL);
if (be_req == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zero failed.\n");
diff --git a/src/providers/dp_backend.h b/src/providers/dp_backend.h
index f90d0b9c5fe69b1b14caa090bb515c60746de154..cfd8b8aa6d296e87685ce0d503af26f83352fdd0
100644
--- a/src/providers/dp_backend.h
+++ b/src/providers/dp_backend.h
@@ -291,9 +291,16 @@ errno_t be_res_init(struct be_ctx *ctx);
/* be_req helpers */
+/* Create a back end request and call fn when done. Please note the
+ * request name is not duplicated. The caller should either provide
+ * a static string or steal a dynamic string onto req context.
+ */
struct be_req *be_req_create(TALLOC_CTX *mem_ctx,
- struct be_client *becli, struct be_ctx *be_ctx,
- be_async_callback_t fn, void *pvt_fn_data);
+ struct be_client *becli,
+ struct be_ctx *be_ctx,
+ const char *name,
+ be_async_callback_t fn,
+ void *pvt_fn_data);
struct be_ctx *be_req_get_be_ctx(struct be_req *be_req);
void *be_req_get_data(struct be_req *be_req);
diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
index 70a2933757688d0cc758a56d20649bf5e7f43436..b849a7b3d0237e53af62d7131f1bf396f64834ca
100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -1342,7 +1342,7 @@ static void ipa_subdom_online_cb(void *pvt)
refresh_interval = ctx->be_ctx->domain->subdomain_refresh_interval;
- be_req = be_req_create(ctx, NULL, ctx->be_ctx,
+ be_req = be_req_create(ctx, NULL, ctx->be_ctx, "IPA subdomains",
ipa_subdom_be_req_callback, NULL);
if (be_req == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "be_req_create() failed.\n");
--
2.4.3
From d7b00e1ef4984bb17d4ce62b2c206efebb6aea37 Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Wed, 11 Nov 2015 13:40:16 +0100
Subject: [PATCH 2/2] DP: Reduce code duplication in Data Provider handlers
Instead of setting the three same variables over again, add a structure
be_sbus_reply_data with a default initializer BE_SBUS_REPLY_DATA_INIT.
The handlers can then set the structure to BE_SBUS_REPLY_DATA_INIT on
declaration or set a particular value with be_sbus_reply_data_set.
The handler can also reply to the message (typically on failure state)
with be_sbus_req_reply_data()
---
src/providers/data_provider_be.c | 270 +++++++++++++++------------------------
1 file changed, 102 insertions(+), 168 deletions(-)
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index b67efa87008d8e15fb9fb27c3d02996608b0aa62..fa9e86ca6e7d02d6f9f348c0a2054ee69f962f45
100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -342,6 +342,40 @@ static errno_t be_offline_reply(struct sbus_request **sbus_req_ptr)
return ret;
}
+struct be_sbus_reply_data {
+ dbus_uint16_t err_maj;
+ dbus_uint32_t err_min;
+ const char *err_msg;
+};
+
+#define BE_SBUS_REPLY_DATA_INIT { .err_maj = DP_ERR_FATAL, \
+ .err_min = ERR_INTERNAL, \
+ .err_msg = "Fatal error" \
+ };
+
+static inline void be_sbus_reply_data_set(struct be_sbus_reply_data *rdata,
+ dbus_uint16_t err_maj,
+ dbus_uint32_t err_min,
+ const char *err_msg)
+{
+ if (rdata == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Bug: Attempt to set NULL be_sbus_reply_data\n");
+ return;
+ }
+
+ rdata->err_maj = err_maj;
+ rdata->err_min = err_min;
+ rdata->err_msg = err_msg;
+}
+
+static inline errno_t be_sbus_req_reply_data(struct sbus_request *sbus_req,
+ struct be_sbus_reply_data *data)
+{
+ return be_sbus_reply(sbus_req, data->err_maj,
+ data->err_min, data->err_msg);
+}
+
void be_terminate_domain_requests(struct be_ctx *be_ctx,
const char *domain)
{
@@ -762,9 +796,7 @@ static int be_get_subdomains(struct sbus_request *dbus_req, void
*user_data)
struct be_req *be_req = NULL;
struct be_client *becli;
char *domain_hint;
- dbus_uint16_t err_maj;
- dbus_uint32_t err_min;
- const char *err_msg;
+ struct be_sbus_reply_data req_reply = BE_SBUS_REPLY_DATA_INIT;
int ret;
becli = talloc_get_type(user_data, struct be_client);
@@ -777,10 +809,8 @@ static int be_get_subdomains(struct sbus_request *dbus_req, void
*user_data)
/* return an error if corresponding backend target is not configured */
if (becli->bectx->bet_info[BET_SUBDOMAINS].bet_ops == NULL) {
- DEBUG(SSSDBG_TRACE_INTERNAL, "Undefined backend target.\n");
- err_maj = DP_ERR_FATAL;
- err_min = ENODEV;
- err_msg = "Subdomains back end target is not configured";
+ be_sbus_reply_data_set(&req_reply, DP_ERR_FATAL, ENODEV,
+ "Subdomains back end target is not
configured");
goto immediate;
}
@@ -791,9 +821,8 @@ static int be_get_subdomains(struct sbus_request *dbus_req, void
*user_data)
*/
if (becli->bectx->offstat.offline) {
DEBUG(SSSDBG_TRACE_FUNC, "Cannot proceed, provider is offline.\n");
I wanted to push patches but I moticed that DEBUG message was removed
in previous case "Undefined backend target", but was not removed here.
If we want to remove them, please remove them on all places.
If we do not want to touch them, do not remove it in previous case.
- err_maj = DP_ERR_OFFLINE;
- err_min = EAGAIN;
- err_msg = "Provider is offline";
+ be_sbus_reply_data_set(&req_reply, DP_ERR_OFFLINE, EAGAIN,
+ "Provider is offline");
goto immediate;
}
LS