Merge pull request #38 from jfsmig/M-fixes-20190826

jfsmig · web-flow · commit d8104ecf028b · 2019-08-27T09:46:41.000+02:00
Various fixes
diff --git a/main/children.c b/main/children.c
@@ -357,7 +357,14 @@ _child_exec(struct child_s *sd, int argc, char ** args)
 	/* If the target command is just a filename, then try to find
 	 * it in the PATH that could have been set for this command */
 	env = _child_build_env(sd);
-	supervisor_children_cleanall();
+
+	/* JFS: No need to free the allocated memory of gridinit, execve will do. */
+	/* supervisor_children_cleanall(); */
+
+	/* JFS: the internal sockets of libdill do not carry the O_CLOEXEC flag.
+	 *      We want to avoid any FD leak to the children. */
+	for (int i=3; i<8; i++)
+		close(i);
 
 	if (g_path_is_absolute(cmd))
 		real_cmd = g_strdup(cmd);
@@ -380,7 +387,7 @@ _child_start(struct child_s *sd, void *udata, supervisor_cb_f cb)
 	typeof(errno) errsav;
 	gint argc;
 	gchar **args;
-	struct my_rlimits_s saved_limits;
+	struct my_rlimits_s saved_limits = {};
 
 	if (!sd || !sd->command) {
 		errno = EINVAL;
@@ -392,8 +399,6 @@ _child_start(struct child_s *sd, void *udata, supervisor_cb_f cb)
 		return -1;
 	}
 
-	bzero(&saved_limits, sizeof(saved_limits));
-
 	sd->last_start_attempt = _monotonic_seconds();
 	sd->last_start = time(0);
 
@@ -782,7 +787,7 @@ supervisor_children_register(const gchar *key, const gchar *cmd)
 		return FALSE;
 	}
 
-	g_strlcpy(sd->key, key, sizeof(sd->key)-1);
+	g_strlcpy(sd->key, key, sizeof(sd->key));
 	sd->delay_before_KILL = supervisor_default_delay_KILL;
 	sd->flags = MASK_STARTED|MASK_RESPAWN|MASK_DELAYED;
 	sd->working_directory = g_get_current_dir();
@@ -1127,9 +1132,11 @@ supervisor_children_set_group(const gchar *key, const gchar *group)
 		return -1;
 	}
 
-	bzero(sd->group, sizeof(sd->group));
-	if (group)
-		g_strlcpy(sd->group, group, sizeof(sd->group)-1);
+	if (group) {
+		g_strlcpy(sd->group, group, sizeof(sd->group));
+	} else {
+		memset(sd->group, 0, sizeof(sd->group));
+	}
 	errno = 0;
 	return 0;
 }
diff --git a/main/gridinit.c b/main/gridinit.c
@@ -512,7 +512,8 @@ _client_run(int ch, int ldh_client)
 static dill_coroutine void
 _server_run(int ch, const char *path)
 {
-	int workers = dill_bundle();
+	struct dill_bundle_storage bundle_storage = {};
+	int workers = dill_bundle_mem(&bundle_storage);
 	g_assert(workers >= 0);
 
 	int ldh_server = dill_ipc_listen(path, 1024);
@@ -525,7 +526,7 @@ _server_run(int ch, const char *path)
 	DEBUG("Initiated a server socket on [%s] h=%d", sock_path, ldh_server);
 
 	while (flag_running) {
-		int ldh_client = dill_ipc_accept(ldh_server, dill_now() + 30000);
+		int ldh_client = dill_ipc_accept(ldh_server, dill_now() + 1000);
 		if (ldh_client >= 0) {
 			TRACE("Client accepted h=%d", ldh_client);
 			dill_bundle_go(workers, _client_run(ch, ldh_client));
@@ -538,8 +539,10 @@ _server_run(int ch, const char *path)
 	int rc = dill_hclose(ldh_server);
 	g_assert(rc == 0);
 
-	dill_bundle_wait(workers, dill_now() + 30000);
+	dill_bundle_wait(workers, -1);
 	dill_hclose(workers);
+
+	unlink(path);
 }
 
 static void
@@ -759,9 +762,7 @@ _group_is_accepted(gchar *str_key, gchar *str_group)
 static gboolean
 _service_exists(const gchar *key)
 {
-	struct child_info_s ci;
-
-	bzero(&ci, sizeof(ci));
+	struct child_info_s ci = {};
 	return 0 == supervisor_children_get_info(key, &ci);
 }
 
@@ -967,39 +968,33 @@ _cfg_section_default(GKeyFile *kf, const gchar *section, GError **err)
 		else if (!g_ascii_strcasecmp(*p_key, CFG_KEY_PATH_WORKINGDIR)) {
 			if (!g_file_test(*p_key, G_FILE_TEST_IS_DIR|G_FILE_TEST_IS_EXECUTABLE))
 				WARN("Default working directory does not exist yet [%s]", *p_key);
-			bzero(default_working_directory, sizeof(default_working_directory));
-			g_strlcpy(default_working_directory, str, sizeof(default_working_directory)-1);
+			g_strlcpy(default_working_directory, str, sizeof(default_working_directory));
 		}
 		else if (!g_ascii_strcasecmp(*p_key, CFG_KEY_PATH_PIDFILE)) {
-			bzero(pidfile_path, sizeof(pidfile_path));
-			g_strlcpy(pidfile_path, str, sizeof(pidfile_path)-1);
+			g_strlcpy(pidfile_path, str, sizeof(pidfile_path));
 		}
 		else if (!g_ascii_strcasecmp(*p_key, CFG_KEY_LISTEN)) {
-			if (str[0] == '/')
+			if (str[0] == '/') {
 				g_strlcpy(sock_path, str, sizeof(sock_path));
-			else
+			} else {
 				g_printerr("section=%s, key=listen : not a UNIX path, ignored! [%s]\n",
 					section, str);
+			}
 		}
 		else if (!g_ascii_strcasecmp(*p_key, CFG_KEY_USER)) {
-			bzero(buf_user, sizeof(buf_user));
-			g_strlcpy(buf_user, str, sizeof(buf_user)-1);
+			g_strlcpy(buf_user, str, sizeof(buf_user));
 		}
 		else if (!g_ascii_strcasecmp(*p_key, CFG_KEY_GROUP)) {
-			bzero(buf_group, sizeof(buf_group));
-			g_strlcpy(buf_group, str, sizeof(buf_group)-1);
+			g_strlcpy(buf_group, str, sizeof(buf_group));
 		}
 		else if (!g_ascii_strcasecmp(*p_key, CFG_KEY_UID)) {
-			bzero(buf_uid, sizeof(buf_uid));
-			g_strlcpy(buf_uid, str, sizeof(buf_uid)-1);
+			g_strlcpy(buf_uid, str, sizeof(buf_uid));
 		}
 		else if (!g_ascii_strcasecmp(*p_key, CFG_KEY_GID)) {
-			bzero(buf_gid, sizeof(buf_gid));
-			g_strlcpy(buf_gid, str, sizeof(buf_gid)-1);
+			g_strlcpy(buf_gid, str, sizeof(buf_gid));
 		}
 		else if (!g_ascii_strcasecmp(*p_key, CFG_KEY_INCLUDES)) {
-			bzero(buf_includes, sizeof(buf_includes));
-			g_strlcpy(buf_includes, str, sizeof(buf_includes)-1);
+			g_strlcpy(buf_includes, str, sizeof(buf_includes));
 		}
 		else if (!g_ascii_strcasecmp(*p_key, CFG_KEY_GROUPSONLY)) {
 			_str_set_array(FALSE, &groups_only_cfg, str);
@@ -1175,11 +1170,7 @@ _cfg_reload(gboolean services_only, GError **err)
 static guint16
 compute_thread_id(GThread *thread)
 {
-	union {
-		void *p;
-		guint16 u[4];
-	} bulk;
-	memset(&bulk, 0, sizeof(bulk));
+	union { void *p; guint16 u[4]; } bulk = {};
 	bulk.p = thread;
 	return (bulk.u[0] ^ bulk.u[1]) ^ (bulk.u[2] ^ bulk.u[3]);
 }
@@ -1395,11 +1386,10 @@ static gboolean
 is_gridinit_running(const gchar *path)
 {
 	int rc, usock;
-	struct sockaddr_un sun;
+	struct sockaddr_un sun = {};
 
-	bzero(&sun, sizeof(sun));
 	sun.sun_family = AF_UNIX;
-	g_strlcpy(sun.sun_path, path, sizeof(sun.sun_path) - 1);
+	g_strlcpy(sun.sun_path, path, sizeof(sun.sun_path));
 
 	if (0 > (usock = socket(PF_UNIX, SOCK_STREAM, 0)))
 		return FALSE;
@@ -1554,6 +1544,9 @@ main(int argc, char ** args)
 	supervisor_children_fini();
 
 	g_free(config_path);
+	if (*pidfile_path) {
+		unlink(pidfile_path);
+	}
 	closelog();
 	return rc;
 }
diff --git a/main/gridinit_cmd.c b/main/gridinit_cmd.c
@@ -31,6 +31,8 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 #include "./gridinit_internals.h"
 
+#define UNUSED __attribute__ ((unused))
+
 #define MINI 0
 #define MEDIUM 1
 
@@ -379,40 +381,40 @@ open_cnx(void)
 static int
 send_commandv(void (*dumper)(FILE *, void*), void *udata, const char *cmd, int argc, char **args)
 {
-	FILE *req_stream;
-	if (NULL != (req_stream = open_cnx())) {
-		fputs(cmd, req_stream);
-		fputc(' ', req_stream);
-		for (int i=0; i<argc ;i++) {
-			fputs(args[i], req_stream);
-			fputc(' ', req_stream);
-		}
-		fputc('\n', req_stream);
-
-		fflush(req_stream);
-		dumper(req_stream, udata);
-		fclose(req_stream);
+	FILE *req_stream = open_cnx();
+	if (!req_stream)
 		return 1;
+
+	fputs(cmd, req_stream);
+	fputc(' ', req_stream);
+	for (int i=0; i<argc ;i++) {
+		fputs(args[i], req_stream);
+		fputc(' ', req_stream);
 	}
+	fputc('\n', req_stream);
 
+	fflush(req_stream);
+	dumper(req_stream, udata);
+	fclose(req_stream);
 	return 0;
 }
 
-static GList *
-_fetch_services(void)
+static int
+_fetch_services(GList **out)
 {
 	GList *jobs = NULL;
-	void _on_reply(FILE *in_stream, void *udata) {
-		(void) udata;
+	void _on_reply(FILE *in_stream, void *udata UNUSED) {
 		jobs = read_services_list(in_stream);
 	}
 
 	int rc = send_commandv(_on_reply, NULL, "status", 0, (char*[]){NULL});
-	if (!rc) {
+	if (rc != 0) {
 		g_list_free_full(jobs, (GDestroyNotify)child_info_free);
-		return NULL;
+		*out = NULL;
+		return 1;
 	} else {
-		return jobs;
+		*out = jobs;
+		return 0;
 	}
 }
 
@@ -454,7 +456,10 @@ command_status(int lvl, int argc, char **args)
 	int *counters = alloca(sizeof(int) * (argc+1));
 	memset(counters, 0, sizeof(int) * (argc+1));
 
-	GList *all_jobs = _fetch_services();
+	GList *all_jobs = NULL;
+	if (0 != _fetch_services(&all_jobs))
+		return 1;
+
 	GList *jobs = _filter_services(all_jobs, args, counters);
 	FORMAT format_t = parse_format(format);
 
@@ -609,7 +614,7 @@ command_start(int argc, char **args)
 	struct dump_as_is_arg_s dump_args = {};
 
 	int rc = send_commandv(dump_as_is, &dump_args, "start", argc, args);
-	return !rc
+	return rc
 		|| dump_args.count_errors != 0
 		|| dump_args.count_success == 0;
 }
@@ -620,35 +625,44 @@ command_kill(int argc, char **args)
 	struct dump_as_is_arg_s dump_args = {};
 
 	int rc = send_commandv(dump_as_is, &dump_args, "stop", argc, args);
-	return !rc
+	return rc
 		|| dump_args.count_errors != 0
 		|| dump_args.count_success == 0;
 }
 
+static gboolean
+_all_down(char **args, gboolean *down)
+{
+	GList *all_jobs = NULL;
+	if (0 != _fetch_services(&all_jobs))
+		return FALSE;
+
+	GList *jobs = _filter_services(all_jobs, args, NULL);
+	for (GList *l = jobs; l ;l=l->next) {
+		struct child_info_s *ci = l->data;
+		if (ci->pid > 0)
+			*down = FALSE;
+	}
+	g_list_free_full(all_jobs, (GDestroyNotify)child_info_free);
+	g_list_free(jobs);
+	return TRUE;
+}
+
 static int
 command_stop(int argc, char **args)
 {
-	gboolean _all_down(void) {
-		gboolean rc = TRUE;
-		GList *all_jobs = _fetch_services();
-		GList *jobs = _filter_services(all_jobs, args, NULL);
-		for (GList *l = jobs; l ;l=l->next) {
-			struct child_info_s *ci = l->data;
-			if (ci->pid > 0)
-				rc = FALSE;
-		}
-		g_list_free_full(all_jobs, (GDestroyNotify)child_info_free);
-		g_list_free(jobs);
-		return rc;
-	}
 	FORMAT format_t = parse_format(format);
-	while (!_all_down()) {
+	for (;;) {
+		gboolean d = TRUE;
+		if (!_all_down(args, &d))
+			return 1;
+		if (d)
+			return 0;
 		/* If standart output format*/
 		if (format_t != DEFAULT)
 			g_print("# Stopping...\n");
-		int rc = command_kill(argc, args);
-		if (rc != 0)
-			return rc;
+		if (0 != command_kill(argc, args))
+			return 1;
 		g_usleep(G_TIME_SPAN_SECOND);
 	}
 	return 0;
@@ -660,7 +674,7 @@ command_restart(int argc, char **args)
 	struct dump_as_is_arg_s dump_args = {};
 
 	int rc = send_commandv(dump_as_is, &dump_args, "restart", argc, args);
-	return !rc
+	return rc
 		|| dump_args.count_errors != 0
 		|| dump_args.count_success == 0;
 }
@@ -671,19 +685,18 @@ command_repair(int argc, char **args)
 	struct dump_as_is_arg_s dump_args = {};
 
 	int rc = send_commandv(dump_as_is, &dump_args, "repair", argc, args);
-	return !rc
+	return rc
 		|| dump_args.count_errors != 0
 		|| dump_args.count_success == 0;
 }
 
 static int
-command_reload(int argc, char **args)
+command_reload(int argc UNUSED, char **args UNUSED)
 {
 	struct dump_as_is_arg_s dump_args = {};
-	(void) argc, (void) args;
 
 	int rc = send_commandv(dump_as_is, &dump_args, "reload", 0, (char*[]){NULL});
-	return !rc
+	return rc
 		|| dump_args.count_errors != 0
 		|| dump_args.count_success == 0;
 }
