Fix the lacks of pthread_rwlock_unlock() calls.

Two pthread_rwlock_unlock() calls are necessary if *cgroup is null
and fprintf()/fflush() fails in cgroup_attach_task_pid():

 src/api.c:785
 785             pthread_rwlock_rdlock(&cg_mount_table_lock);
 786             for(i = 0; i < CG_CONTROLLER_MAX &&
 787                             cg_mount_table[i].name[0]!='\0'; i++) {
 [snip]
 805                     ret = fprintf(tasks, "%d", tid);
 806                     if (ret < 0) {
    <<pthread_rwlock_unlock() call is necessary>>
 807                             cgroup_dbg("Error writing tid %d to
%s:%s\n",
 808                                             tid, path,
strerror(errno));
 809                             fclose(tasks);
 810                             last_errno = errno;
 811                             return ECGOTHER;
 812                     }
 813
 814                     ret = fflush(tasks);
 815                     if (ret) {
    <<pthread_rwlock_unlock() call is necessary>>
 816                             last_errno = errno;
 817                             cgroup_dbg("Error writing tid  %d to
%s:%s\n",
 818                                              tid, path,
strerror(errno));
 819                              fclose(tasks);
 820                              return ECGOTHER;
 821                      }

For the readability, this patch merges almost the same lines into one
function(__cgroup_attach_task_pid()) and adds pthread_rwlock_unlock()
call for the case the function fails.

Signed-off-by: Ken'ichi Ohmichi <oomichi@mxs.nes.nec.co.jp>
Signed-off-by: Dhaval Giani <dhaval@linux.vnet.ibm.com>

diff --git a/src/api.c b/src/api.c
index a45802e529c0f7d6e67134df596be9208f38ce82..99e71feb9ccef33ed794e858bb9b4a6109cf880b 100644 (file)
--- a/src/api.c
+++ b/src/api.c
@@ -762,6 +762,43 @@ char *cg_build_path(char *name, char *path, char *type)
        return path;
 }
 
+static int __cgroup_attach_task_pid(char *path, pid_t tid)
+{
+       int ret = 0;
+       FILE *tasks = NULL;
+
+       tasks = fopen(path, "w");
+       if (!tasks) {
+               switch (errno) {
+               case EPERM:
+                       return ECGROUPNOTOWNER;
+               case ENOENT:
+                       return ECGROUPNOTEXIST;
+               default:
+                       return ECGROUPNOTALLOWED;
+               }
+       }
+       ret = fprintf(tasks, "%d", tid);
+       if (ret < 0) {
+               last_errno = errno;
+               ret = ECGOTHER;
+               goto err;
+       }
+       ret = fflush(tasks);
+       if (ret) {
+               last_errno = errno;
+               ret = ECGOTHER;
+               goto err;
+       }
+       fclose(tasks);
+       return 0;
+err:
+       cgroup_dbg("Error writing tid %d to %s:%s\n",
+                       tid, path, strerror(errno));
+       fclose(tasks);
+       return ret;
+}
+
 /** cgroup_attach_task_pid is used to assign tasks to a cgroup.
  *  struct cgroup *cgroup: The cgroup to assign the thread to.
  *  pid_t tid: The thread to be assigned to the cgroup.
@@ -773,7 +810,6 @@ char *cg_build_path(char *name, char *path, char *type)
 int cgroup_attach_task_pid(struct cgroup *cgroup, pid_t tid)
 {
        char path[FILENAME_MAX];
-       FILE *tasks = NULL;
        int i, ret = 0;
 
        if (!cgroup_initialized) {
@@ -789,37 +825,11 @@ int cgroup_attach_task_pid(struct cgroup *cgroup, pid_t tid)
                                                cg_mount_table[i].name))
                                continue;
                        strncat(path, "/tasks", sizeof(path) - strlen(path));
-
-                       tasks = fopen(path, "w");
-                       if (!tasks) {
-                               pthread_rwlock_unlock(&cg_mount_table_lock);
-                               switch (errno) {
-                               case EPERM:
-                                       return ECGROUPNOTOWNER;
-                               case ENOENT:
-                                       return ECGROUPNOTEXIST;
-                               default:
-                                       return ECGROUPNOTALLOWED;
-                               }
-                       }
-                       ret = fprintf(tasks, "%d", tid);
-                       if (ret < 0) {
-                               cgroup_dbg("Error writing tid %d to %s:%s\n",
-                                               tid, path, strerror(errno));
-                               fclose(tasks);
-                               last_errno = errno;
-                               return ECGOTHER;
-                       }
-
-                       ret = fflush(tasks);
+                       ret = __cgroup_attach_task_pid(path, tid);
                        if (ret) {
-                               last_errno = errno;
-                               cgroup_dbg("Error writing tid  %d to %s:%s\n",
-                                               tid, path, strerror(errno));
-                               fclose(tasks);
-                               return ECGOTHER;
+                               pthread_rwlock_unlock(&cg_mount_table_lock);
+                               return ret;
                        }
-                       fclose(tasks);
                }
                pthread_rwlock_unlock(&cg_mount_table_lock);
        } else {
@@ -835,40 +845,10 @@ int cgroup_attach_task_pid(struct cgroup *cgroup, pid_t tid)
                        if (!cg_build_path(cgroup->name, path,
                                        cgroup->controller[i]->name))
                                continue;
-
                        strncat(path, "/tasks", sizeof(path) - strlen(path));
-
-                       tasks = fopen(path, "w");
-                       if (!tasks) {
-                               cgroup_dbg("fopen failed for %s:%s", path,
-                                                       strerror(errno));
-
-                               switch (errno) {
-                               case EPERM:
-                                       return ECGROUPNOTOWNER;
-                               case ENOENT:
-                                       return ECGROUPNOTEXIST;
-                               default:
-                                       return ECGROUPNOTALLOWED;
-                               }
-                       }
-                       ret = fprintf(tasks, "%d", tid);
-                       if (ret < 0) {
-                               last_errno = errno;
-                               cgroup_dbg("Error writing tid %d to %s:%s\n",
-                                               tid, path, strerror(errno));
-                               fclose(tasks);
-                               return ECGOTHER;
-                       }
-                       ret = fflush(tasks);
-                       if (ret) {
-                               last_errno = errno;
-                               cgroup_dbg("Error writing tid  %d to %s:%s\n",
-                                               tid, path, strerror(errno));
-                               fclose(tasks);
-                               return ECGOTHER;
-                       }
-                       fclose(tasks);
+                       ret = __cgroup_attach_task_pid(path, tid);
+                       if (ret)
+                               return ret;
                }
        }
        return 0;