On 05/06/2010 02:48 PM, Stephen Gallagher wrote:
1) The code seems to be duplicated multiple times. Why common code is
not segmented into a common function?

It is now :)

Dmitri pointed out to me off-list that there was a much larger section of redundant code I could turn into a common function.

New patch attached.

--
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
From 22adbd834499b9bc99aef2db8be5aaf462e5f218 Mon Sep 17 00:00:00 2001
From: Stephen Gallagher <sgall...@redhat.com>
Date: Thu, 6 May 2010 11:23:23 -0400
Subject: [PATCH] Fix segfault in GSSAPI reconnect code

Also clean up some duplicated code into a single common routine
sdap_account_info_common_done()
---
 src/providers/ldap/ldap_id.c |  120 ++++++++++++++++++++++-------------------
 1 files changed, 64 insertions(+), 56 deletions(-)

diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index c472d8b6439514b027b4627f397777933c79db01..d12e9d7b1d269e69a94d310448ea48eb51819a5c 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -724,96 +724,104 @@ static int sdap_account_info_restart(struct be_req *breq)
     return EOK;
 }
 
+enum account_restart {
+    SDAP_ACCOUNT_DO_NOT_RESTART,
+    SDAP_ACCOUNT_DO_RESTART
+};
+
+int sdap_account_info_common_done(int ret, struct be_req *breq, int *dp_err)
+{
+    struct sdap_id_ctx *ctx;
+    enum account_restart err = SDAP_ACCOUNT_DO_NOT_RESTART;
+
+    if (ret != EOK) {
+        *dp_err = DP_ERR_FATAL;
+
+        if (ret == ETIMEDOUT || ret == EFAULT || ret == EIO) {
+            *dp_err = DP_ERR_OFFLINE;
+            ctx = talloc_get_type(breq->be_ctx->bet_info[BET_ID].pvt_bet_data,
+                                  struct sdap_id_ctx);
+            if (sdap_check_gssapi_reconnect(ctx)) {
+                if (ctx->gsh) {
+                    /* Mark the connection as false so we don't try to use an
+                     * invalid connection by mistake later.
+                     * If the global sdap handler is NULL, it's ok not to do
+                     * anything here. It's always checked by sdap_connected()
+                     * before being used.
+                     */
+                    ctx->gsh->connected = false;
+                }
+                err = sdap_account_info_restart(breq);
+                if (err == EOK) return SDAP_ACCOUNT_DO_RESTART;
+            }
+            sdap_mark_offline(ctx);
+        }
+    }
+
+    return err;
+}
+
 static void sdap_account_info_users_done(struct tevent_req *req)
 {
     struct be_req *breq = tevent_req_callback_data(req, struct be_req);
-    struct sdap_id_ctx *ctx;
     int dp_err = DP_ERR_OK;
-    const char *error = NULL;
-    int ret, err;
+    const char *errstr = NULL;
+    int ret;
+    enum account_restart err;
 
     ret = users_get_recv(req);
     talloc_zfree(req);
 
-    if (ret) {
-        dp_err = DP_ERR_FATAL;
-        error = "Enum Users Failed";
-
-        if (ret == ETIMEDOUT || ret == EFAULT || ret == EIO) {
-            dp_err = DP_ERR_OFFLINE;
-            ctx = talloc_get_type(breq->be_ctx->bet_info[BET_ID].pvt_bet_data,
-                                  struct sdap_id_ctx);
-            if (sdap_check_gssapi_reconnect(ctx)) {
-                ctx->gsh->connected = false;
-                err = sdap_account_info_restart(breq);
-                if (err == EOK) return;
-            }
-            sdap_mark_offline(ctx);
-        }
+    err = sdap_account_info_common_done(ret, breq, &dp_err);
+    if (err == SDAP_ACCOUNT_DO_RESTART) {
+        /* Restarting the connection */
+        return;
     }
 
-    sdap_handler_done(breq, dp_err, ret, error);
+    /* No more connections to try */
+    if (ret != EOK) errstr = "Enum Users Failed";
+    sdap_handler_done(breq, dp_err, ret, errstr);
 }
 
 static void sdap_account_info_groups_done(struct tevent_req *req)
 {
     struct be_req *breq = tevent_req_callback_data(req, struct be_req);
-    struct sdap_id_ctx *ctx;
     int dp_err = DP_ERR_OK;
-    const char *error = NULL;
+    const char *errstr = NULL;
     int ret, err;
 
     ret = groups_get_recv(req);
     talloc_zfree(req);
 
-    if (ret) {
-        dp_err = DP_ERR_FATAL;
-        error = "Enum Groups Failed";
-
-        if (ret == ETIMEDOUT || ret == EFAULT || ret == EIO) {
-            dp_err = DP_ERR_OFFLINE;
-            ctx = talloc_get_type(breq->be_ctx->bet_info[BET_ID].pvt_bet_data,
-                                  struct sdap_id_ctx);
-            if (sdap_check_gssapi_reconnect(ctx)) {
-                ctx->gsh->connected = false;
-                err = sdap_account_info_restart(breq);
-                if (err == EOK) return;
-            }
-            sdap_mark_offline(ctx);
-        }
+    err = sdap_account_info_common_done(ret, breq, &dp_err);
+    if (err == SDAP_ACCOUNT_DO_RESTART) {
+        /* Restarting the connection */
+        return;
     }
 
-    return sdap_handler_done(breq, dp_err, ret, error);
+    /* No more connections to try */
+    if (ret != EOK) errstr = "Enum Groups Failed";
+    return sdap_handler_done(breq, dp_err, ret, errstr);
 }
 
 static void sdap_account_info_initgr_done(struct tevent_req *req)
 {
     struct be_req *breq = tevent_req_callback_data(req, struct be_req);
-    struct sdap_id_ctx *ctx;
     int dp_err = DP_ERR_OK;
-    const char *error = NULL;
-    int ret;
+    const char *errstr = NULL;
+    int ret, err;
 
     ret = groups_by_user_recv(req);
     talloc_zfree(req);
 
-    if (ret) {
-        dp_err = DP_ERR_FATAL;
-        error = "Init Groups Failed";
-
-        if (ret == ETIMEDOUT || ret == EFAULT || ret == EIO) {
-            dp_err = DP_ERR_OFFLINE;
-            ctx = talloc_get_type(breq->be_ctx->bet_info[BET_ID].pvt_bet_data,
-                                  struct sdap_id_ctx);
-            if (sdap_check_gssapi_reconnect(ctx)) {
-                ctx->gsh->connected = false;
-                sdap_account_info_restart(breq);
-                return;
-            }
-            sdap_mark_offline(ctx);
-        }
+    err = sdap_account_info_common_done(ret, breq, &dp_err);
+    if (err == SDAP_ACCOUNT_DO_RESTART) {
+        /* Restarting the connection */
+        return;
     }
 
-    return sdap_handler_done(breq, dp_err, ret, error);
+    /* No more connections to try */
+    if (ret != EOK) errstr = "Init Groups Failed";
+    return sdap_handler_done(breq, dp_err, ret, errstr);
 }
 
-- 
1.7.0.1

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to