afs: Fix a shm leak.

Even if the command handler stops to read the command output
afs must continue to read shmids from server fd so that it can
destroy all shared memory areas that the server created for afs.

diff --git a/afs.c b/afs.c
index b8acb53a9ffa2782e8fc0421a9a01f5642347eed..fd705b1fa824013c702df97eefdce420bb02022f 100644 (file)
--- a/afs.c
+++ b/afs.c
@@ -141,6 +141,36 @@ struct callback_result {
        size_t result_size;
 };
 
+static int dispatch_result(int result_shmid, callback_result_handler *handler,
+               void *private_result_data)
+{
+       struct osl_object result;
+       void *result_shm;
+       int ret2, ret = shm_attach(result_shmid, ATTACH_RO, &result_shm);
+       struct callback_result *cr = result_shm;
+
+       if (ret < 0) {
+               PARA_ERROR_LOG("attach failed: %s\n", para_strerror(-ret));
+               return ret;
+       }
+       result.size = cr->result_size;
+       result.data = result_shm + sizeof(*cr);
+       if (result.size) {
+               assert(handler);
+               ret = handler(&result, private_result_data);
+               if (ret < 0)
+                       PARA_NOTICE_LOG("result handler error: %s\n",
+                               para_strerror(-ret));
+       }
+       ret2 = shm_detach(result_shm);
+       if (ret2 < 0) {
+               PARA_ERROR_LOG("detach failed: %s\n", para_strerror(-ret2));
+               if (ret >= 0)
+                       ret = ret2;
+       }
+       return ret;
+}
+
 /**
  * Ask the afs process to call a given function.
  *
@@ -161,8 +191,7 @@ struct callback_result {
  * shmid are passed to that function as an osl object. The private_result_data
  * pointer is passed as the second argument to \a result_handler.
  *
- * \return Negative, on errors, the return value of the callback function
- * otherwise.
+ * \return Standard.
  *
  * \sa send_option_arg_callback_request(), send_standard_callback_request().
  */
@@ -171,10 +200,11 @@ int send_callback_request(callback_function *f, struct osl_object *query,
                void *private_result_data)
 {
        struct callback_query *cq;
-       int num_results = 0, ret, fd = -1, query_shmid, result_shmid;
-       void *query_shm, *result_shm;
+       int ret, fd = -1, query_shmid, result_shmid;
+       void *query_shm;
        char buf[sizeof(afs_socket_cookie) + sizeof(int)];
        size_t query_shm_size = sizeof(*cq);
+       int dispatch_error = 0;
 
        if (query)
                query_shm_size += query->size;
@@ -205,45 +235,37 @@ int send_callback_request(callback_function *f, struct osl_object *query,
        ret = send_bin_buffer(fd, buf, sizeof(buf));
        if (ret < 0)
                goto out;
+       /*
+        * Read all shmids from afs.
+        *
+        * Even if the dispatcher returns an error we _must_ continue to read
+        * shmids from fd so that we can destroy all shared memory areas that
+        * have been created for us by the afs process.
+        */
        for (;;) {
                ret = recv_bin_buffer(fd, buf, sizeof(int));
                if (ret <= 0)
                        goto out;
-               if (ret != sizeof(int)) {
-                       ret = -E_AFS_SHORT_READ;
-                       goto out;
-               }
+               assert(ret == sizeof(int));
                ret = *(int *) buf;
-               if (ret <= 0)
-                       goto out;
+               assert(ret > 0);
                result_shmid = ret;
-               ret = shm_attach(result_shmid, ATTACH_RO, &result_shm);
-               if (ret >= 0) {
-                       struct callback_result *cr = result_shm;
-                       struct osl_object result;
-                       num_results++;
-                       result.size = cr->result_size;
-                       result.data = result_shm + sizeof(*cr);
-                       if (result.size) {
-                               assert(result_handler);
-                               ret = result_handler(&result, private_result_data);
-                               if (shm_detach(result_shm) < 0)
-                                       PARA_ERROR_LOG("can not detach result\n");
-                       }
-               } else
-                       PARA_ERROR_LOG("attach result failed: %d\n", ret);
-               if (shm_destroy(result_shmid) < 0)
-                       PARA_ERROR_LOG("destroy result failed\n");
+               if (!dispatch_error) {
+                       ret = dispatch_result(result_shmid, result_handler,
+                               private_result_data);
+                       if (ret < 0)
+                               dispatch_error = 1;
+               }
+               ret = shm_destroy(result_shmid);
                if (ret < 0)
-                       break;
+                       PARA_CRIT_LOG("destroy result failed: %s\n",
+                               para_strerror(-ret));
        }
 out:
        if (shm_destroy(query_shmid) < 0)
-               PARA_ERROR_LOG("%s\n", "shm destroy error");
+               PARA_CRIT_LOG("shm destroy error\n");
        if (fd >= 0)
                close(fd);
-       if (ret >= 0)
-               ret = num_results;
 //     PARA_DEBUG_LOG("callback_ret: %d\n", ret);
        return ret;
 }