[SSSD] [PATCH 6/6] Fix segfault in GSSAPI reconnect code

Dmitri Pal dpal at redhat.com
Thu May 6 22:33:44 UTC 2010


Stephen Gallagher wrote:
> 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.
>

Sorry but let me be a bit picky
+    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;


how about the following approach:




+void sdap_account_info_common_done(int ret, struct be_req *breq, const
char *errstr)
+{
+    struct sdap_id_ctx *ctx;
+    int err = DP_ERR_OK;
+    int error = EOK;
+
+    if (ret != EOK) {
+        err = DP_ERR_FATAL;
+
+        if (ret == ETIMEDOUT || ret == EFAULT || ret == EIO) {
+            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;
+                }
+                error = sdap_account_info_restart(breq);
+                if (!error) return;
+            }
+            sdap_mark_offline(ctx);
+        }
+        sdap_handler_done(breq, err, ret, errstr);
+    }
+    else sdap_handler_done(breq, DP_ERR_OK, EOK, NULL);
+}
+
 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 = "Enum Users Failed";
+    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);
-        }
-    }
-    sdap_handler_done(breq, dp_err, ret, error);
+    sdap_account_info_common_done(ret, breq, errstr);

 }

And then you do not need enum that looks a bit ugly here...

At least this gives the idea of what I mean...



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


-- 
Thank you,
Dmitri Pal

Engineering Manager IPA project,
Red Hat Inc.


-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/




More information about the sssd-devel mailing list