Misc fixes, plugged a memory leak in runscript.c and use va_copy to avoid nasty segfaults

src/Makefile

@@ -30,11 +30,11 @@ LIBRCOBJS= librc.o librc-depend.o librc-daemon.o librc-misc.o librc-strlist.o
 
 LIB_TARGETS = $(LIBEINFOSO) $(LIBRCSO)
 BIN_TARGETS = rc-status
-SBIN_TARGETS = env-update fstabinfo mountinfo \
+SBIN_TARGETS = rc rc-update runscript start-stop-daemon
-	       rc rc-depend rc-update runscript start-stop-daemon
+PRIV_BIN_TARGETS = env-update fstabinfo mountinfo rc-depend
 SYS_WHITELIST = env_whitelist
 
-TARGET = $(LIB_TARGETS) $(BIN_TARGETS) $(SBIN_TARGETS)
+TARGET = $(LIB_TARGETS) $(BIN_TARGETS) $(SBIN_TARGETS) $(PRIV_BIN_TARGETS)
 
 RCLINKS = einfon einfo ewarnn ewarn eerrorn eerror ebegin eend ewend \
 	      eindent eoutdent eflush color_terminal \
@@ -142,6 +142,7 @@ install: $(TARGET)
 	install -m 0755 -d $(DESTDIR)/$(LIB)/rcscripts/conf.d
 	install -m 0644 $(SYS_WHITELIST) $(DESTDIR)/$(LIB)/rcscripts/conf.d
 	install -m 0755 -d $(DESTDIR)/$(LIB)/rcscripts/bin
+	install -m 0755 $(PRIV_BIN_TARGETS) $(DESTDIR)/$(LIB)/rcscripts/bin
 	for x in $(RCLINKS); do ln -sf $(DESTDIR)/sbin/rc $(DESTDIR)/$(LIB)/rcscripts/bin/$$x; done
 	if test "$(HAVE_PAM)" != "" ; then \
 		install -m 0755 -d $(DESTDIR)/etc/pam.d ; \

src/env-update.c

@@ -134,10 +134,9 @@ int main (int argc, char **argv)
 		{
 		  if (isspecial)
 		    {
-		      envs[k - 1] = rc_xrealloc (envs[k - 1],
+		      int len =  strlen (envs[k - 1]) + strlen (entry) + 1;
-						 strlen (envs[k - 1]) +
+		      envs[k - 1] = rc_xrealloc (envs[k - 1], len);
-						 strlen (entry) + 1);
+		      snprintf (envs[k - 1] + strlen (envs[k - 1]), len,
-		      sprintf (envs[k - 1] + strlen (envs[k - 1]),
 			       "%s%s", isspecial_spaced ? " " : ":", value);
 		    }
 		  else

src/libeinfo.c

@@ -92,9 +92,9 @@ static bool is_env (const char *var, const char *val)
 
   v = getenv (var);
   if (! v)
-    return (val == NULL ? true : false);
+    return (val ? false : true);
 
-  return (strcasecmp (v, val) == 0 ? true : false);
+  return (strcasecmp (v, val) ? false : true);
 }
 
 bool colour_terminal (void)
@@ -164,8 +164,11 @@ static int ebuffer (const char *cmd, int retval, const char *fmt, va_list ap)
 
   if (fmt)
     {
-      l = vsnprintf (buffer, sizeof (buffer), fmt, ap);
+      va_list apc;
+      va_copy (apc, ap);
+      l = vsnprintf (buffer, sizeof (buffer), fmt, apc);
       fprintf (fp, "%d %s\n", l, buffer);
+      va_end (apc);
     }
   else
     fprintf (fp, "0\n");
@@ -351,12 +354,16 @@ void eflush (void)
 static void elog (int level, const char *fmt, va_list ap)
 {
   char *e = getenv ("RC_ELOG");
+  va_list apc;
 
-  if (e)
+  if (fmt && e)
     {
       closelog ();
       openlog (e, LOG_PID, LOG_DAEMON);
-      vsyslog (level, fmt, ap);
+      va_copy (apc, ap);
+      vsyslog (level, fmt, apc);
+      va_end (apc);
+      closelog ();
     }
 }
 static int _eindent (FILE *stream)
@@ -390,7 +397,12 @@ static int _eindent (FILE *stream)
   else \
     fprintf (_file, " * "); \
   retval += _eindent (_file); \
-  retval += vfprintf (_file, fmt, ap) + 3; \
+  { \
+    va_list _ap; \
+    va_copy (_ap, ap); \
+    retval += vfprintf (_file, fmt, _ap) + 3; \
+    va_end (_ap); \
+  } \
   if (colour_terminal ()) \
     fprintf (_file, "\033[K");
 
@@ -626,22 +638,32 @@ static int _do_eend (const char *cmd, int retval, const char *fmt, va_list ap)
 {
   int col = 0;
   FILE *fp;
+  va_list apc;
+  int eb;
 
-  if (ebuffer (cmd, retval, fmt, ap))
+  if (fmt)
-    return (retval);
+    {
+      va_copy (apc, ap);
+      eb = ebuffer (cmd, retval, fmt, apc);
+      va_end (apc);
+      if (eb)
+	return (retval);
+    }
 
   if (fmt && retval != 0)
     {
+      va_copy (apc, ap);
       if (strcmp (cmd, "ewend") == 0)
 	{
-	  col = _vewarnn (fmt, ap);
+	  col = _vewarnn (fmt, apc);
 	  fp = stdout;
 	}
       else
 	{
-	  col = _veerrorn (fmt, ap);
+	  col = _veerrorn (fmt, apc);
 	  fp = stderr;
 	}
+      va_end (apc);
       if (colour_terminal ())
 	fprintf (fp, "\n");
     }

src/librc-daemon.c

@@ -448,8 +448,9 @@ bool rc_service_started_daemon (const char *service, const char *exec,
 
   if (indx > 0)
     {
-      file = rc_xmalloc (sizeof (char *) * 10);
+      int len = sizeof (char *) * 10;
-      snprintf (file, sizeof (file), "%03d", indx);
+      file = rc_xmalloc (len);
+      snprintf (file, len, "%03d", indx);
       retval = _match_daemon (dirpath, file, mexec, NULL, NULL);
       free (file);
     }

src/librc-depend.c

@@ -630,7 +630,7 @@ int rc_update_deptree (bool force)
     setenv ("RC_LIBDIR", RC_LIBDIR, 0);
 
   /* Phase 1 */
-  if ((fp = popen (GENDEP, "r")) == NULL)
+  if (! (fp = popen (GENDEP, "r")))
     eerrorx ("popen: %s", strerror (errno));
 
   deptree = rc_xmalloc (sizeof (rc_depinfo_t));
@@ -809,7 +809,7 @@ int rc_update_deptree (bool force)
      This works and should be entirely shell parseable provided that depend
      names don't have any non shell variable characters in
      */
-  if ((fp = fopen (RC_DEPTREE, "w")) == NULL)
+  if (! (fp = fopen (RC_DEPTREE, "w")))
     eerror ("fopen `%s': %s", RC_DEPTREE, strerror (errno));
   else
     {

src/librc-misc.c

@@ -695,16 +695,16 @@ char **rc_config_env (char **env)
 	{
 	  fclose (fp);
 	  if (file_regex ("/proc/xen/capabilities", "control_d"))
-	    sprintf (sys, "XENU");
+	    snprintf (sys, sizeof (sys), "XENU");
 	}
       if (! sys)
-	sprintf (sys, "XEN0");
+	snprintf (sys, sizeof (sys), "XEN0");
     }
   else if (file_regex ("/proc/cpuinfo", "UML"))
-    sprintf (sys, "UML");
+    snprintf (sys, sizeof (sys), "UML");
   else if (file_regex ("/proc/self/status",
 		       "(s_context|VxID|envID):[[:space:]]*[1-9]"))
-    sprintf(sys, "VPS"); 
+    snprintf (sys, sizeof (sys), "VPS"); 
 #endif
 
   /* Only add a NET_FS list if not defined */

src/librc-strlist.c

@@ -49,7 +49,7 @@ static char **_rc_strlist_addsort (char **list, const char *item,
 
   newlist = rc_xrealloc (list, sizeof (char *) * (i + 2));
 
-  if (i == 0)
+  if (! i)
     newlist[i] = NULL;
   newlist[i + 1] = NULL;
 
@@ -88,7 +88,7 @@ char **rc_strlist_delete (char **list, const char *item)
     return (list);
 
   while (list[i])
-    if (strcmp (list[i], item) == 0)
+    if (! strcmp (list[i], item))
       {
 	free (list[i]);
 	do

src/rc-plugin.c

@@ -47,6 +47,7 @@ void rc_plugin_load (void)
       void *h = dlopen (p, RTLD_LAZY);
       char *func;
       void *f;
+      int len;
 
       if (! h)
 	{
@@ -57,8 +58,9 @@ void rc_plugin_load (void)
 
       func = file;
       file = strsep (&func, ".");
-      func = rc_xmalloc (strlen (file) + strlen ("__hook") + 1);
+      len = strlen (file) + 7;
-      sprintf (func, "_%s_hook", file);
+      func = rc_xmalloc (sizeof (char *) * len);
+      snprintf (func, len, "_%s_hook", file);
 
       f = dlsym (h, func);
       if (! f)

src/rc.c

@@ -1013,19 +1013,22 @@ int main (int argc, char **argv)
       /* Unless we would use a different config file */
       if (found)
 	{
+	  int len;
 	  if (! newlevel)
 	    continue;
-	  
+
-	  tmp = rc_xmalloc (strlen (service) + strlen (runlevel) + 2);
+	  len = strlen (service) + strlen (runlevel) + 2;
-	  sprintf (tmp, "%s.%s", service, runlevel);
+	  tmp = rc_xmalloc (sizeof (char *) * len);
+	  snprintf (tmp, len, "%s.%s", service, runlevel);
 	  conf = rc_strcatpaths (RC_CONFDIR, tmp, (char *) NULL);
 	  found = rc_exists (conf);
 	  CHAR_FREE (conf);
 	  CHAR_FREE (tmp);
 	  if (! found)
 	    {
-	      tmp = rc_xmalloc (strlen (service) + strlen (newlevel) + 2);
+	      len = strlen (service) + strlen (newlevel) + 2;
-	      sprintf (tmp, "%s.%s", service, newlevel);
+	      tmp = rc_xmalloc (sizeof (char *) * len);
+	      snprintf (tmp, len, "%s.%s", service, newlevel);
 	      conf = rc_strcatpaths (RC_CONFDIR, tmp, (char *) NULL);
 	      found = rc_exists (conf);
 	      CHAR_FREE (conf);

src/runscript.c

@@ -136,7 +136,7 @@ static time_t get_mtime (const char *pathname, bool follow_link)
     return (0);
 
   retval = follow_link ? stat (pathname, &buf) : lstat (pathname, &buf);
-  if (retval == 0)
+  if (! retval)
     return (buf.st_mtime);
 
   errno = 0;
@@ -154,13 +154,13 @@ static bool in_control ()
   if (sighup)
     return (false);
 
-  if (mtime_test == NULL || ! rc_exists (mtime_test))
+  if (! mtime_test || ! rc_exists (mtime_test))
     return (false);
 
   if (rc_service_state (applet, rc_service_stopped))
     return (false);
 
-  if ((mtime = get_mtime (mtime_test, false)) == 0)
+  if (! (mtime = get_mtime (mtime_test, false)))
     return (false);
 
   while (tests[i])
@@ -214,6 +214,8 @@ static void cleanup (void)
     rc_strlist_free (restart_services);
   if (need_services)
     rc_strlist_free (need_services);
+  if (tmplist)
+    rc_strlist_free (tmplist);
   if (mycmd)
     free (mycmd);
   if (myarg1)
@@ -312,7 +314,7 @@ static bool svc_exec (const char *service, const char *arg1, const char *arg2)
   signal (SIGCHLD, handle_signal);
 
   if (WIFEXITED (status))
-    return (WEXITSTATUS (status) == 0 ? true : false);
+    return (WEXITSTATUS (status) ? false : true);
 
   return (false);
 }
@@ -514,8 +516,7 @@ static void svc_start (const char *service, bool deps)
 	  if (rc_service_state (svc, rc_service_started))
 	    continue;
 	  if (! rc_wait_service (svc))
-	    { eerror ("%s: timed out waiting for %s", applet, svc);
+	    eerror ("%s: timed out waiting for %s", applet, svc);
-	      system ("ps ax > /tmp/$SVCNAME.waiting"); }
 	  if (rc_service_state (svc, rc_service_started))
 	    continue;
 
@@ -560,18 +561,19 @@ static void svc_start (const char *service, bool deps)
 	      n++;
 	    }
 
-	  tmp = rc_xmalloc (sizeof (char *) * len + 5);
+	  len += 5;
+	  tmp = rc_xmalloc (sizeof (char *) * len);
 	  p = tmp;
 	  STRLIST_FOREACH (tmplist, svc, i)
 	    {
 	      if (i > 1)
 		{
 		  if (i == n - 1)
-		    p += sprintf (p, " or ");
+		    p += snprintf (p, len, " or ");
 		  else
-		    p += sprintf (p, ", ");
+		    p += snprintf (p, len, ", ");
 		}
-	      p += sprintf (p, "%s", svc);
+	      p += snprintf (p, len, "%s", svc);
 	    }
 	  ewarnx ("WARNING: %s is scheduled to start when %s has started",
 		  applet, tmp);

src/strlist.h

@@ -18,7 +18,7 @@
    it should usually by +1 from what you expect, and should only be
    used in the scope of the macro) */
 #define STRLIST_FOREACH(_list, _pos, _counter) \
- if ((_list) && _list[0] && ((_counter = 0) == 0)) \
+ if ((_list) && _list[0] && ! (_counter = 0)) \
    while ((_pos = _list[_counter++]))
 
 #endif /* __STRLIST_H__ */