Don't be an ass - don't free the 2nd list. Instead just empty it.

7 files changed, 55 insertions, 35 deletions

diff --git a/ChangeLog b/ChangeLog
index 7fd85f29..cbe91069 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -12,8 +12,7 @@
     rc_strlist_add and friends now take char *** instead of char ** and
     return a pointer to the item added instead of the new list head. This is
     so we can easily tell if the item was successfully added or not instead
-    of iterating through the list looking for it. rc_strlist_join now frees
-    list2 for ease of use.
+    of iterating through the list looking for it. 
 	
     list = rc_strlist_add (list, item);
     becomes
diff --git a/src/librc-depend.c b/src/librc-depend.c
index 32bbf2bd..4df64635 100644
--- a/src/librc-depend.c
+++ b/src/librc-depend.c
@@ -479,6 +479,7 @@ char **rc_order_services (rc_depinfo_t *deptree, const char *runlevel,
 	char **types = NULL;
 	char **services = NULL;
 	bool reverse = false;
+	char **tmp = NULL;
 
 	if (! runlevel)
 		return (NULL);
@@ -493,25 +494,33 @@ char **rc_order_services (rc_depinfo_t *deptree, const char *runlevel,
 		strcmp (runlevel, RC_LEVEL_REBOOT) == 0)
 	{
 		list = rc_ls_dir (RC_SVCDIR_STARTING, RC_LS_INITD);
-		rc_strlist_join (&list,
-						 rc_ls_dir (RC_SVCDIR_INACTIVE, RC_LS_INITD));
-		rc_strlist_join (&list,
-						 rc_ls_dir (RC_SVCDIR_STARTED, RC_LS_INITD));
+
+		tmp = rc_ls_dir (RC_SVCDIR_INACTIVE, RC_LS_INITD);
+		rc_strlist_join (&list, tmp);
+		rc_strlist_free (tmp);
+
+		tmp = rc_ls_dir (RC_SVCDIR_INACTIVE, RC_LS_INITD);
+		rc_strlist_join (&list, tmp);
+		rc_strlist_free (tmp);
 		reverse = true;
 	} else {
 		list = rc_services_in_runlevel (runlevel);
 
 		/* Add coldplugged services */
-		rc_strlist_join (&list, rc_ls_dir (RC_SVCDIR_COLDPLUGGED, RC_LS_INITD));
+		tmp = rc_ls_dir (RC_SVCDIR_COLDPLUGGED, RC_LS_INITD);
+		rc_strlist_join (&list, tmp);
+		rc_strlist_free (tmp);
 
 		/* If we're not the boot runlevel then add that too */
 		if (strcmp (runlevel, bootlevel) != 0) {
 			char *path = rc_strcatpaths (RC_RUNLEVELDIR, bootlevel,
 										 (char *) NULL);
-			rc_strlist_join (&list, rc_ls_dir (path, RC_LS_INITD));
+			tmp = rc_ls_dir (path, RC_LS_INITD);
+			rc_strlist_join (&list, tmp);
+			rc_strlist_free (tmp);
 			free (path);
 		}
-	} 
+	}
 
 	/* Now we have our lists, we need to pull in any dependencies
 	   and order them */
diff --git a/src/librc-misc.c b/src/librc-misc.c
index 9bac7902..4c40cfbf 100644
--- a/src/librc-misc.c
+++ b/src/librc-misc.c
@@ -462,7 +462,10 @@ char **rc_filter_env (void)
 	if (! whitelist)
 		ewarn ("system environment whitelist (" SYS_WHITELIST ") missing");
 
-	rc_strlist_join (&whitelist, rc_get_list (USR_WHITELIST));
+	env = rc_get_list (USR_WHITELIST);
+	rc_strlist_join (&whitelist, env);
+	rc_strlist_free (env);
+	env = NULL;
 
 	if (! whitelist)
 		return (NULL);
diff --git a/src/librc-strlist.c b/src/librc-strlist.c
index dc9bc20c..14d08baa 100644
--- a/src/librc-strlist.c
+++ b/src/librc-strlist.c
@@ -142,16 +142,10 @@ int rc_strlist_join (char ***list1, char **list2)
 	int i = 0;
 	int j = 0;
 
-	if (! lst1 && list2) {
-		*list1 = list2;
-		return (0);
-	}
-	if (! list2 && lst1)
-		return (0);
-	if (! lst1 && ! list2)
+	if (! list2)
 		return (0);
 
-	while (lst1[i])
+	while (lst1 && lst1[i])
 		i++;
 
 	while (list2[j])
@@ -162,14 +156,13 @@ int rc_strlist_join (char ***list1, char **list2)
 	j = 0;
 	while (list2[j]) {
 		newlist[i] = list2[j];
+		/* Take the item off the 2nd list as it's only a shallow copy */
+		list2[j] = NULL;
 		i++;
 		j++;
 	}
 	newlist[i] = NULL;
 
-	/* We free list2 here for ease of use. */
-	free (list2);
-	
 	*list1 = newlist;
 	return (0);
 }
diff --git a/src/rc.c b/src/rc.c
index 561d895c..49b53bed 100644
--- a/src/rc.c
+++ b/src/rc.c
@@ -723,6 +723,7 @@ int main (int argc, char **argv)
 	char *newlevel = NULL;
 	char *service = NULL;
 	char **deporder = NULL;
+	char **tmplist;
 	int i = 0;
 	int j = 0;
 	bool going_down = false;
@@ -815,7 +816,9 @@ int main (int argc, char **argv)
 	/* Ensure our environment is pure
 	   Also, add our configuration to it */
 	env = rc_filter_env ();
-	rc_strlist_join (&env, rc_make_env ());
+	tmplist = rc_make_env ();
+	rc_strlist_join (&env, tmplist);
+	rc_strlist_free (tmplist);
 
 	if (env) {
 		char *p;
@@ -1089,17 +1092,23 @@ int main (int argc, char **argv)
 	/* Build a list of all services to stop and then work out the
 	   correct order for stopping them */
 	stop_services = rc_ls_dir (RC_SVCDIR_STARTING, RC_LS_INITD);
-	rc_strlist_join (&stop_services,
-					 rc_ls_dir (RC_SVCDIR_INACTIVE, RC_LS_INITD));
-	rc_strlist_join (&stop_services,
-					 rc_ls_dir (RC_SVCDIR_STARTED, RC_LS_INITD));
+	
+	tmplist = rc_ls_dir (RC_SVCDIR_INACTIVE, RC_LS_INITD);
+	rc_strlist_join (&stop_services, tmplist);
+	rc_strlist_free (tmplist);
+
+	tmplist = rc_ls_dir (RC_SVCDIR_STARTED, RC_LS_INITD);
+	rc_strlist_join (&stop_services, tmplist);
+	rc_strlist_free (tmplist);
 
 	types = NULL;
 	rc_strlist_add (&types, "ineed");
 	rc_strlist_add (&types, "iuse");
 	rc_strlist_add (&types, "iafter");
+
 	deporder = rc_get_depends (deptree, types, stop_services,
 							   runlevel, depoptions | RC_DEP_STOP);
+
 	rc_strlist_free (stop_services);
 	rc_strlist_free (types);
 	types = NULL;
@@ -1123,7 +1132,9 @@ int main (int argc, char **argv)
 		}
 		tmp = rc_strcatpaths (RC_RUNLEVELDIR, newlevel ? newlevel : runlevel,
 							  (char *) NULL);
-		rc_strlist_join (&start_services, rc_ls_dir (tmp, RC_LS_INITD));
+		tmplist = rc_ls_dir (tmp, RC_LS_INITD);
+		rc_strlist_join (&start_services, tmplist);
+		rc_strlist_free (tmplist);
 		CHAR_FREE (tmp);
 	} else {
 		/* Store our list of coldplugged services */
@@ -1134,11 +1145,13 @@ int main (int argc, char **argv)
 			strcmp (newlevel ? newlevel : runlevel, RC_LEVEL_REBOOT) != 0)
 		{
 			/* We need to include the boot runlevel services if we're not in it */
-			rc_strlist_join (&start_services,
-							 rc_services_in_runlevel (bootlevel));
-			rc_strlist_join (&start_services,
-							 rc_services_in_runlevel (newlevel ?
-													  newlevel : runlevel));
+			tmplist = rc_services_in_runlevel (bootlevel);
+			rc_strlist_join (&start_services, tmplist);
+			rc_strlist_free (tmplist);
+			tmplist = rc_services_in_runlevel (newlevel ? newlevel : runlevel);
+			rc_strlist_join (&start_services, tmplist);
+			rc_strlist_free (tmplist);
+			
 			STRLIST_FOREACH (coldplugged_services, service, i)
 				rc_strlist_add (&start_services, service);
 
diff --git a/src/rc.h b/src/rc.h
index b43d2d29..5e5e100a 100644
--- a/src/rc.h
+++ b/src/rc.h
@@ -203,8 +203,8 @@ char *rc_strlist_addsort (char ***list, const char *item);
 char *rc_strlist_addsortc (char ***list, const char *item);
 char *rc_strlist_addsortu (char ***list, const char *item);
 int rc_strlist_delete (char ***list, const char *item);
-/* rc_strlist_list_join does a shallow copy of list2 onto list1
- * and then frees the pointer for list2 (but not the contents) */
+/* join moves items from list2 to list1, so list2 is empty
+ * on return. It still needs to be freed though. */
 int rc_strlist_join (char ***list1, char **list2);
 void rc_strlist_reverse (char **list);
 void rc_strlist_free (char **list);
diff --git a/src/runscript.c b/src/runscript.c
index f7d49fb7..d80d1b81 100644
--- a/src/runscript.c
+++ b/src/runscript.c
@@ -1048,8 +1048,11 @@ int runscript (int argc, char **argv)
 	if ((softlevel = getenv ("RC_SOFTLEVEL")) == NULL) {
 		/* Ensure our environment is pure
 		   Also, add our configuration to it */
+		tmplist = rc_make_env();
 		env = rc_filter_env ();
-		rc_strlist_join (&env, rc_make_env ());
+		rc_strlist_join (&env, tmplist);
+		rc_strlist_free (tmplist);
+		tmplist = NULL;
 
 		if (env) {
 			char *p;