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>

1 files changed, 43 insertions, 63 deletions

diff --git a/src/api.c b/src/api.c
index a45802e..99e71fe 100644
--- 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;