cgroups: fix cgroup_event_listener error handling

The error handling in cgroup_event_listener.c did not correctly deal with either an error opening either <control_file> or cgroup.event_control. Due to an uninitialized variable the program exit code was undefined if either of these opens failed. This patch simplifies and corrects cgroup_event_listener.c error handling by: 1. using err*() rather than printf(),exit() 2. depending on process exit to close open files With this patch failures always return non-zero error. Signed-off-by: Greg Thelen <gthelen@google.com> Acked-by: Li Zefan <lizefan@huawei.com> Signed-off-by: Tejun Heo <tj@kernel.org>

cgroups: fix cgroup_event_listener error handling
The error handling in cgroup_event_listener.c did not correctly deal with either an error opening either <control_file> or cgroup.event_control. Due to an uninitialized variable the program exit code was undefined if either of these opens failed. This patch simplifies and corrects cgroup_event_listener.c error handling by: 1. using err*() rather than printf(),exit() 2. depending on process exit to close open files With this patch failures always return non-zero error. Signed-off-by: Greg Thelen <gthelen@google.com> Acked-by: Li Zefan <lizefan@huawei.com> Signed-off-by: Tejun Heo <tj@kernel.org>
Greg Thelen · Tejun Heo
1 parent 92e015b1cf
Showing 1 changed file with 22 additions and 50 deletions Side-by-side Diff
tools/cgroup/cgroup_event_listener.c
@@ -5,6 +5,7 @@
  */
  
 #include <assert.h>
+#include <err.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <libgen.h>
@@ -15,7 +16,7 @@
  
 #include <sys/eventfd.h>
  
-#define USAGE_STR "Usage: cgroup_event_listener <path-to-control-file> <args>\n"
+#define USAGE_STR "Usage: cgroup_event_listener <path-to-control-file> <args>"
  
 int main(int argc, char **argv)
 {
  
  
  
  
  
  
@@ -26,49 +27,33 @@
 	char line[LINE_MAX];
 	int ret;
  
-	if (argc != 3) {
-		fputs(USAGE_STR, stderr);
-		return 1;
-	}
+	if (argc != 3)
+		errx(1, "%s", USAGE_STR);
  
 	cfd = open(argv[1], O_RDONLY);
-	if (cfd == -1) {
-		fprintf(stderr, "Cannot open %s: %s\n", argv[1],
-				strerror(errno));
-		goto out;
-	}
+	if (cfd == -1)
+		err(1, "Cannot open %s", argv[1]);
  
 	ret = snprintf(event_control_path, PATH_MAX, "%s/cgroup.event_control",
 			dirname(argv[1]));
-	if (ret >= PATH_MAX) {
-		fputs("Path to cgroup.event_control is too long\n", stderr);
-		goto out;
-	}
+	if (ret >= PATH_MAX)
+		errx(1, "Path to cgroup.event_control is too long");
  
 	event_control = open(event_control_path, O_WRONLY);
-	if (event_control == -1) {
-		fprintf(stderr, "Cannot open %s: %s\n", event_control_path,
-				strerror(errno));
-		goto out;
-	}
+	if (event_control == -1)
+		err(1, "Cannot open %s", event_control_path);
  
 	efd = eventfd(0, 0);
-	if (efd == -1) {
-		perror("eventfd() failed");
-		goto out;
-	}
+	if (efd == -1)
+		err(1, "eventfd() failed");
  
 	ret = snprintf(line, LINE_MAX, "%d %d %s", efd, cfd, argv[2]);
-	if (ret >= LINE_MAX) {
-		fputs("Arguments string is too long\n", stderr);
-		goto out;
-	}
+	if (ret >= LINE_MAX)
+		errx(1, "Arguments string is too long");
  
 	ret = write(event_control, line, strlen(line) + 1);
-	if (ret == -1) {
-		perror("Cannot write to cgroup.event_control");
-		goto out;
-	}
+	if (ret == -1)
+		err(1, "Cannot write to cgroup.event_control");
  
 	while (1) {
 		uint64_t result;
  
  
  
@@ -77,35 +62,22 @@
 		if (ret == -1) {
 			if (errno == EINTR)
 				continue;
-			perror("Cannot read from eventfd");
-			break;
+			err(1, "Cannot read from eventfd");
 		}
 		assert(ret == sizeof(result));
  
 		ret = access(event_control_path, W_OK);
 		if ((ret == -1) && (errno == ENOENT)) {
-				puts("The cgroup seems to have removed.");
-				ret = 0;
-				break;
-		}
-
-		if (ret == -1) {
-			perror("cgroup.event_control "
-					"is not accessible any more");
+			puts("The cgroup seems to have removed.");
 			break;
 		}
  
+		if (ret == -1)
+			err(1, "cgroup.event_control is not accessible any more");
+
 		printf("%s %s: crossed\n", argv[1], argv[2]);
 	}
  
-out:
-	if (efd >= 0)
-		close(efd);
-	if (event_control >= 0)
-		close(event_control);
-	if (cfd >= 0)
-		close(cfd);
-
-	return (ret != 0);
+	return 0;
 }
...	...	@@ -5,6 +5,7 @@
5	5	*/
6	6
7	7	#include <assert.h>
	8	+#include <err.h>
8	9	#include <errno.h>
9	10	#include <fcntl.h>
10	11	#include <libgen.h>
...	...	@@ -15,7 +16,7 @@
15	16
16	17	#include <sys/eventfd.h>
17	18
18		-#define USAGE_STR "Usage: cgroup_event_listener <path-to-control-file> <args>\n"
	19	+#define USAGE_STR "Usage: cgroup_event_listener <path-to-control-file> <args>"
19	20
20	21	int main(int argc, char **argv)
21	22	{
22	23
23	24
24	25
25	26
26	27
27	28
...	...	@@ -26,49 +27,33 @@
26	27	char line[LINE_MAX];
27	28	int ret;
28	29
29		- if (argc != 3) {
30		- fputs(USAGE_STR, stderr);
31		- return 1;
32		- }
	30	+ if (argc != 3)
	31	+ errx(1, "%s", USAGE_STR);
33	32
34	33	cfd = open(argv[1], O_RDONLY);
35		- if (cfd == -1) {
36		- fprintf(stderr, "Cannot open %s: %s\n", argv[1],
37		- strerror(errno));
38		- goto out;
39		- }
	34	+ if (cfd == -1)
	35	+ err(1, "Cannot open %s", argv[1]);
40	36
41	37	ret = snprintf(event_control_path, PATH_MAX, "%s/cgroup.event_control",
42	38	dirname(argv[1]));
43		- if (ret >= PATH_MAX) {
44		- fputs("Path to cgroup.event_control is too long\n", stderr);
45		- goto out;
46		- }
	39	+ if (ret >= PATH_MAX)
	40	+ errx(1, "Path to cgroup.event_control is too long");
47	41
48	42	event_control = open(event_control_path, O_WRONLY);
49		- if (event_control == -1) {
50		- fprintf(stderr, "Cannot open %s: %s\n", event_control_path,
51		- strerror(errno));
52		- goto out;
53		- }
	43	+ if (event_control == -1)
	44	+ err(1, "Cannot open %s", event_control_path);
54	45
55	46	efd = eventfd(0, 0);
56		- if (efd == -1) {
57		- perror("eventfd() failed");
58		- goto out;
59		- }
	47	+ if (efd == -1)
	48	+ err(1, "eventfd() failed");
60	49
61	50	ret = snprintf(line, LINE_MAX, "%d %d %s", efd, cfd, argv[2]);
62		- if (ret >= LINE_MAX) {
63		- fputs("Arguments string is too long\n", stderr);
64		- goto out;
65		- }
	51	+ if (ret >= LINE_MAX)
	52	+ errx(1, "Arguments string is too long");
66	53
67	54	ret = write(event_control, line, strlen(line) + 1);
68		- if (ret == -1) {
69		- perror("Cannot write to cgroup.event_control");
70		- goto out;
71		- }
	55	+ if (ret == -1)
	56	+ err(1, "Cannot write to cgroup.event_control");
72	57
73	58	while (1) {
74	59	uint64_t result;
75	60
76	61
77	62
...	...	@@ -77,35 +62,22 @@
77	62	if (ret == -1) {
78	63	if (errno == EINTR)
79	64	continue;
80		- perror("Cannot read from eventfd");
81		- break;
	65	+ err(1, "Cannot read from eventfd");
82	66	}
83	67	assert(ret == sizeof(result));
84	68
85	69	ret = access(event_control_path, W_OK);
86	70	if ((ret == -1) && (errno == ENOENT)) {
87		- puts("The cgroup seems to have removed.");
88		- ret = 0;
89		- break;
90		- }
91		-
92		- if (ret == -1) {
93		- perror("cgroup.event_control "
94		- "is not accessible any more");
	71	+ puts("The cgroup seems to have removed.");
95	72	break;
96	73	}
97	74
	75	+ if (ret == -1)
	76	+ err(1, "cgroup.event_control is not accessible any more");
	77	+
98	78	printf("%s %s: crossed\n", argv[1], argv[2]);
99	79	}
100	80
101		-out:
102		- if (efd >= 0)
103		- close(efd);
104		- if (event_control >= 0)
105		- close(event_control);
106		- if (cfd >= 0)
107		- close(cfd);
108		-
109		- return (ret != 0);
	81	+ return 0;
110	82	}