From 984070b03417b64dce968b8c9074a3cf187b3fa7 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher Date: Sun, 8 Mar 2009 18:06:42 -0400 Subject: Improve error handling and replies in the InfoPipe When detecting an internal error in the InfoPipe, make a best- attempt at sending an error message back to the calling program, instead of simply leaving the client to wait for the timeout. --- server/infopipe/infopipe.c | 6 ++++ server/infopipe/infopipe_groups.c | 35 ++++++++++---------- server/infopipe/infopipe_users.c | 69 ++++++++++++++++++++++----------------- 3 files changed, 62 insertions(+), 48 deletions(-) (limited to 'server/infopipe') diff --git a/server/infopipe/infopipe.c b/server/infopipe/infopipe.c index 4ec971caa..8d4bfeefa 100644 --- a/server/infopipe/infopipe.c +++ b/server/infopipe/infopipe.c @@ -144,11 +144,15 @@ static int infp_monitor_init(struct infp_ctx *infp_ctx) /* Helper function to return an immediate error message in the event * of internal error in the InfoPipe to avoid forcing the clients to * time out waiting for a reply. + * This function will make a best effort to send a reply, but if it + * fails, clients will simply have to handle the timeout. */ void infp_return_failure(struct infp_req_ctx *infp_req, const char *message) { DBusMessage *reply; + if(infp_req == NULL) return; + reply = dbus_message_new_error(infp_req->req_message, DBUS_ERROR_FAILED, message); @@ -172,6 +176,8 @@ void infp_return_success(struct infp_req_ctx *infp_req) { DBusMessage *reply; + if (infp_req == NULL) return; + reply = dbus_message_new_method_return(infp_req->req_message); /* If the reply was NULL, we ran out of memory, so we won't * bother trying to queue the message to send. In this case, diff --git a/server/infopipe/infopipe_groups.c b/server/infopipe/infopipe_groups.c index 9aaecf2f2..4b2c881c2 100644 --- a/server/infopipe/infopipe_groups.c +++ b/server/infopipe/infopipe_groups.c @@ -59,9 +59,12 @@ static void infp_do_group_create_callback(void *pvt, int status, reply = dbus_message_new_error(grcreate_req->infp_req->req_message, DBUS_ERROR_FILE_EXISTS, error_msg); + if (reply) + sbus_conn_send_reply(grcreate_req->infp_req->sconn, reply); + } + else { + infp_return_failure(grcreate_req->infp_req, NULL); } - if (reply) - sbus_conn_send_reply(grcreate_req->infp_req->sconn, reply); talloc_free(grcreate_req); return; } @@ -75,9 +78,8 @@ static void infp_do_group_create_callback(void *pvt, int status, /* We have no more usernames to add, so commit the transaction */ sysdb_transaction_done(grcreate_req->sysdb_req, status); - reply = - dbus_message_new_method_return(grcreate_req->infp_req->req_message); - if (reply) sbus_conn_send_reply(grcreate_req->infp_req->sconn, reply); + + infp_return_success(grcreate_req->infp_req); talloc_free(grcreate_req); return; } @@ -97,6 +99,7 @@ static void infp_do_group_create(struct sysdb_req *req, void *pvt) if (ret != EOK) { DEBUG(0, ("Could not invoke sysdb_add_group\n")); sysdb_transaction_done(grcreate_req->sysdb_req, ret); + infp_return_failure(grcreate_req->infp_req, NULL); talloc_free(grcreate_req); return; } @@ -221,6 +224,7 @@ einval: error: if (arg_grnames) dbus_free_string_array(arg_grnames); + if(grcreate_req) infp_return_failure(grcreate_req->infp_req, NULL); talloc_free(grcreate_req); return ret; } @@ -234,7 +238,6 @@ struct infp_deletegroup_ctx { static void infp_do_group_delete_callback(void *pvt, int status, struct ldb_result *res) { - DBusMessage *reply = NULL; struct infp_deletegroup_ctx *grdel_req = talloc_get_type(pvt, struct infp_deletegroup_ctx); @@ -244,15 +247,12 @@ static void infp_do_group_delete_callback(void *pvt, int status, if (status != EOK) { DEBUG(0, ("Failed to delete group from sysdb. Error code %d\n", status)); + infp_return_failure(grdel_req->infp_req, NULL); talloc_free(grdel_req); return; } - reply = dbus_message_new_method_return(grdel_req->infp_req->req_message); - if (reply) { - sbus_conn_send_reply(grdel_req->infp_req->sconn, reply); - dbus_message_unref(reply); - } + infp_return_success(grdel_req->infp_req); talloc_free(grdel_req); } @@ -270,6 +270,7 @@ static void infp_do_group_delete(struct sysdb_req *req, void *pvt) grdel_req); if (ret != EOK) { DEBUG(0, ("Could not delete group entry\n")); + infp_return_failure(grdel_req->infp_req, NULL); talloc_free(grdel_req); return; } @@ -381,6 +382,7 @@ einval: return EOK; error: + if (grdel_req) infp_return_failure(grdel_req->infp_req, NULL); talloc_free(grdel_req); return ret; } @@ -437,11 +439,7 @@ static void infp_do_member_callback(void *pvt, int status, fail: sysdb_transaction_done(grmod_req->sysdb_req, status); fail_msg = talloc_asprintf(grmod_req, "Could not modify group"); - reply = dbus_message_new_error(grmod_req->infp_req->req_message, - DBUS_ERROR_FAILED, - fail_msg); - sbus_conn_send_reply(grmod_req->infp_req->sconn, reply); - dbus_message_unref(reply); + infp_return_failure(grmod_req->infp_req, fail_msg); talloc_free(grmod_req); return; } @@ -492,6 +490,7 @@ static void infp_do_member(struct sysdb_req *req, void *pvt) return; error: + infp_return_failure(grmod_req->infp_req, NULL); talloc_free(grmod_req); return; } @@ -722,6 +721,7 @@ static void infp_do_gid(struct sysdb_req *req, void *pvt) error_msg); if (reply) sbus_conn_send_reply(grmod_req->infp_req->sconn, reply); } + infp_return_failure(grmod_req->infp_req, NULL); talloc_free(grmod_req); return; } @@ -831,8 +831,7 @@ einval: return EOK; error: + if(grmod_req) infp_return_failure(grmod_req->infp_req, NULL); talloc_free(grmod_req); return ret; - - } diff --git a/server/infopipe/infopipe_users.c b/server/infopipe/infopipe_users.c index 119c66fcc..196080905 100644 --- a/server/infopipe/infopipe_users.c +++ b/server/infopipe/infopipe_users.c @@ -52,6 +52,7 @@ static void infp_users_get_cached_callback(void *ptr, if (status != LDB_SUCCESS) { DEBUG(0, ("Failed to enumerate users in the cache db.\n")); + infp_return_failure(infp_getcached_req->infp_req, NULL); talloc_free(infp_getcached_req); return; } @@ -59,6 +60,7 @@ static void infp_users_get_cached_callback(void *ptr, /* Construct a reply */ reply = dbus_message_new_method_return(infp_getcached_req->infp_req->req_message); if(reply == NULL) { + infp_return_failure(infp_getcached_req->infp_req, NULL); talloc_free(infp_getcached_req); return; } @@ -93,6 +95,7 @@ error: ("Critical error constructing reply message for %s\n", INFP_USERS_GET_CACHED)); dbus_message_unref(reply); + infp_return_failure(infp_getcached_req->infp_req, NULL); talloc_free(infp_getcached_req); return; } @@ -185,6 +188,8 @@ einval: return EOK; error: + if (infp_getcached_req) + infp_return_failure(infp_getcached_req->infp_req, NULL); talloc_free(infp_getcached_req); return ret; } @@ -217,7 +222,7 @@ static void infp_do_user_create_callback(void *pvt, */ if (status == EOK) { /* Return reply ack */ - reply = dbus_message_new_method_return(infp_createuser_req->infp_req->req_message); + infp_return_success(infp_createuser_req->infp_req); } else if (status == EEXIST) { /* Return error, user already exists */ @@ -228,18 +233,17 @@ static void infp_do_user_create_callback(void *pvt, reply = dbus_message_new_error(infp_createuser_req->infp_req->req_message, DBUS_ERROR_FILE_EXISTS, error_msg); + if (reply) { + sbus_conn_send_reply(infp_createuser_req->infp_req->sconn, reply); + dbus_message_unref(reply); + } } else { /* Unknown error occurred. Print DEBUG message */ DEBUG(0, ("Failed to create user in the sysdb. Error code %d\n", status)); - talloc_free(infp_createuser_req); - return; + infp_return_failure(infp_createuser_req->infp_req, NULL); } - if (reply) { - sbus_conn_send_reply(infp_createuser_req->infp_req->sconn, reply); - dbus_message_unref(reply); - } talloc_free(infp_createuser_req); } @@ -261,6 +265,7 @@ static void infp_do_user_create(struct sysdb_req *req, void *pvt) if (ret != EOK) { DEBUG(0, ("Could not invoke sysdb_add_user\n")); sysdb_transaction_done(infp_createuser_req->sysdb_req, ret); + infp_return_failure(infp_createuser_req->infp_req, NULL); talloc_free(infp_createuser_req); return; } @@ -399,6 +404,8 @@ einval: return EOK; error: + if(infp_createuser_req) + infp_return_failure(infp_createuser_req->infp_req, NULL); talloc_free(infp_createuser_req); return ret; } @@ -413,7 +420,6 @@ struct infp_deleteuser_ctx { static void infp_do_user_delete_callback(void *pvt, int status, struct ldb_result *res) { - DBusMessage *reply = NULL; struct infp_deleteuser_ctx *infp_deleteuser_req = talloc_get_type(pvt, struct infp_deleteuser_ctx); @@ -422,16 +428,12 @@ static void infp_do_user_delete_callback(void *pvt, int status, if (status != EOK) { DEBUG(0, ("Failed to delete user from sysdb. Error code %d\n", status)); + infp_return_failure(infp_deleteuser_req->infp_req, NULL); talloc_free(infp_deleteuser_req); return; } - reply = dbus_message_new_method_return(infp_deleteuser_req->infp_req->req_message); - if(reply) { - sbus_conn_send_reply(infp_deleteuser_req->infp_req->sconn, - reply); - dbus_message_unref(reply); - } + infp_return_success(infp_deleteuser_req->infp_req); talloc_free(infp_deleteuser_req); } @@ -448,6 +450,7 @@ static void infp_do_user_delete(struct sysdb_req *req, void *pvt) infp_deleteuser_req->username); if(infp_deleteuser_req->user_dn == NULL) { DEBUG(0, ("Could not construct a user_dn for deletion.\n")); + infp_return_failure(infp_deleteuser_req->infp_req, NULL); talloc_free(infp_deleteuser_req); return; } @@ -458,6 +461,7 @@ static void infp_do_user_delete(struct sysdb_req *req, void *pvt) infp_deleteuser_req); if(ret != EOK) { DEBUG(0,("Could not delete user entry.\n")); + infp_return_failure(infp_deleteuser_req->infp_req, NULL); talloc_free(infp_deleteuser_req); return; } @@ -576,6 +580,8 @@ einval: return EOK; error: + if(infp_deleteuser_req) + infp_return_failure(infp_deleteuser_req->infp_req, NULL); talloc_free(infp_deleteuser_req); return ret; } @@ -888,6 +894,7 @@ static void infp_get_attr_lookup_callback(void *ptr, int ldb_status, struct ldb_ /* Process the current results */ if (ldb_status != LDB_SUCCESS) { DEBUG(0, ("Critical error reading from sysdb.\n")); + infp_return_failure(infp_getattr_req->infp_req, NULL); goto done; } @@ -908,6 +915,7 @@ static void infp_get_attr_lookup_callback(void *ptr, int ldb_status, struct ldb_ default: DEBUG(0, ("GetUser call returned more than one result. This probably means the sysdb is corrupt!\n")); + infp_return_failure(infp_getattr_req->infp_req, NULL); goto done; } } @@ -928,12 +936,14 @@ static void infp_get_attr_lookup_callback(void *ptr, int ldb_status, struct ldb_ &infp_getattr_req->results[infp_getattr_req->index]); if (ret != EOK) { DEBUG(0, ("Unable to create result map!\n")); + infp_return_failure(infp_getattr_req->infp_req, NULL); goto done; } break; default: /* We received more than one result. This is bad */ DEBUG(0, ("GetUser call returned more than one result. This probably means the sysdb is corrupt!\n")); + infp_return_failure(infp_getattr_req->infp_req, NULL); goto done; } @@ -943,6 +953,7 @@ static void infp_get_attr_lookup_callback(void *ptr, int ldb_status, struct ldb_ ret = infp_get_attr_lookup(infp_getattr_req); if (ret != EOK) { DEBUG(0, ("Could not read from cache database\n")); + infp_return_failure(infp_getattr_req->infp_req, NULL); goto done; } return; @@ -951,6 +962,7 @@ static void infp_get_attr_lookup_callback(void *ptr, int ldb_status, struct ldb_ /* No more names remain, return the result DICTs */ reply = dbus_message_new_method_return(infp_getattr_req->infp_req->req_message); if (reply == NULL) { + infp_return_failure(infp_getattr_req->infp_req, NULL); goto done; } @@ -1262,6 +1274,7 @@ end: dbus_free_string_array(usernames); dbus_free_string_array(attributes); if (ret != EOK) { + infp_return_failure(infp_getattr_req->infp_req, NULL); talloc_free(infp_getattr_req); } return ret; @@ -1288,7 +1301,6 @@ struct infp_setattr_ctx { static void infp_do_user_set_attr(struct sysdb_req *req, void *pvt); static void infp_do_user_set_attr_callback(void *ptr, int ldb_status, struct ldb_result *res) { - DBusMessage *reply; struct infp_setattr_ctx *infp_setattr_req; infp_setattr_req = talloc_get_type(ptr, struct infp_setattr_ctx); @@ -1298,6 +1310,7 @@ static void infp_do_user_set_attr_callback(void *ptr, int ldb_status, struct ldb DEBUG(0, ("Failed to store user attributes to the sysdb\n")); /* Cancel the transaction */ sysdb_transaction_done(infp_setattr_req->sysdb_req, sysdb_error_to_errno(ldb_status)); + infp_return_failure(infp_setattr_req->infp_req, NULL); talloc_free(infp_setattr_req); return; } @@ -1313,13 +1326,8 @@ static void infp_do_user_set_attr_callback(void *ptr, int ldb_status, struct ldb sysdb_transaction_done(infp_setattr_req->sysdb_req, EOK); /* Send reply ack */ - reply = dbus_message_new_method_return(infp_setattr_req->infp_req->req_message); - if(reply == NULL) { - talloc_free(infp_setattr_req); - return; - } - sbus_conn_send_reply(infp_setattr_req->infp_req->sconn, reply); - dbus_message_unref(reply); + infp_return_success(infp_setattr_req->infp_req); + talloc_free(infp_setattr_req); } @@ -1341,6 +1349,7 @@ static void infp_do_user_set_attr(struct sysdb_req *req, void *pvt) if(ret != EOK) { DEBUG(0, ("Failed to set attributes for user [%s]. Cancelling transaction\n", infp_setattr_req->usernames[infp_setattr_req->index])); sysdb_transaction_done(req, ret); + infp_return_failure(infp_setattr_req->infp_req, NULL); talloc_free(infp_setattr_req); } } @@ -1653,6 +1662,8 @@ einval: return EOK; error: + if(infp_setattr_req) + infp_return_failure(infp_setattr_req->infp_req, NULL); talloc_free(infp_setattr_req); return ret; } @@ -1666,7 +1677,6 @@ struct infp_setuid_ctx { static void infp_do_user_set_uid_callback(void *ptr, int ldb_status, struct ldb_result *res) { - DBusMessage *reply; struct infp_setuid_ctx *infp_setuid_req = talloc_get_type(ptr, struct infp_setuid_ctx); /* Commit or cancel the transaction, based on the ldb_status */ @@ -1675,18 +1685,14 @@ static void infp_do_user_set_uid_callback(void *ptr, int ldb_status, struct ldb_ /* Check the LDB result */ if (ldb_status != LDB_SUCCESS) { DEBUG(0, ("Failed to store user uid to the sysdb\n")); + infp_return_failure(infp_setuid_req->infp_req, NULL); talloc_free(infp_setuid_req); return; } /* Send reply ack */ - reply = dbus_message_new_method_return(infp_setuid_req->infp_req->req_message); - if(reply == NULL) { - talloc_free(infp_setuid_req); - return; - } - sbus_conn_send_reply(infp_setuid_req->infp_req->sconn, reply); - dbus_message_unref(reply); + infp_return_success(infp_setuid_req->infp_req); + talloc_free(infp_setuid_req); } @@ -1708,6 +1714,7 @@ static void infp_do_user_set_uid(struct sysdb_req *req, void *pvt) if (ret != EOK) { DEBUG(0, ("Could not invoke sysdb_set_user_attr")); sysdb_transaction_done(infp_setuid_req->sysdb_req, ret); + infp_return_failure(infp_setuid_req->infp_req, NULL); talloc_free(infp_setuid_req); return; } @@ -1826,6 +1833,8 @@ einval: return EOK; error: + if(infp_setuid_req) + infp_return_failure(infp_setuid_req->infp_req, NULL); talloc_free(infp_setuid_req); return ret; } -- cgit