Cleanup and refactor of public plugin API

Improve upon API names, both internal and external, to make it clearer
where methods are located and clarify responsibility.

Add new Makefile target "dev" to create untagged development snapshots.

Bump version for upcoming release.

Signed-off-by: Joachim Nilsson <troglobit@gmail.com>

Makefile

@@ -29,9 +29,10 @@
 ROOTDIR    ?= $(shell pwd)
 
 #VERSION    ?= $(shell git tag -l | tail -1)
-VERSION    ?= 1.3
+VERSION    ?= 1.4
 EXEC        = finit
 PKG         = $(EXEC)-$(VERSION)
+DEV         = $(EXEC)-dev
 ARCHIVE     = $(PKG).tar.xz
 HEADERS     = plugin.h svc.h helpers.h
 DISTFILES   = LICENSE README ChangeLog finit.conf services
@@ -139,6 +140,15 @@ dist:
 	git archive --format=tar --prefix=$(PKG)/ $(VERSION) | xz >../$(ARCHIVE)
 	@(cd ..; md5sum $(ARCHIVE) | tee $(ARCHIVE).md5)
 
+dev: distclean
+	@echo "Building unstable xz $(DEV) in parent dir..."
+	-@$(RM) -f ../$(DEV).tar.xz*
+	@(dir=`mktemp -d`; mkdir $$dir/$(DEV); cp -a . $$dir/$(DEV); \
+	  cd $$dir; tar --exclude=.git --exclude=contrib             \
+                        -c -J -f $(DEV).tar.xz $(DEV);               \
+	  cd - >/dev/null; mv $$dir/$(DEV).tar.xz ../; cd ..;        \
+	  rm -rf $$dir; md5sum $(DEV).tar.xz | tee $(DEV).tar.xz.md5)
+
 # Include automatically generated rules, such as:
 # uncgi.o: .../some/dir/uncgi.c /usr/include/stdio.h
 # but don't bother during clean!

finit.c

@@ -64,7 +64,7 @@ static int run_loop(void)
 {
 	while (1) {
 		svc_monitor();
-		io_monitor();
+		plugin_monitor();
 	}
 
 	return 0;
@@ -141,10 +141,10 @@ int main(int UNUSED(args), char *argv[])
 	 * Load plugins and run first level hooks.
 	 */
 	_d("Loading plugins ...");
-	load_plugins(PLUGIN_PATH);
+	plugin_load_all(PLUGIN_PATH);
 
 	_d("Running first level hooks ...");
-	run_hooks(HOOK_BASEFS_UP);
+	plugin_run_hooks(HOOK_BASEFS_UP);
 
 	/*
 	 * Network stuff
@@ -164,7 +164,7 @@ int main(int UNUSED(args), char *argv[])
 	/*
 	 * Hooks that rely on loopback, or basic networking being up.
 	 */
-	run_hooks(HOOK_NETWORK_UP);
+	plugin_run_hooks(HOOK_NETWORK_UP);
 
 	/*
 	 * Start service monitor framework
@@ -181,7 +181,7 @@ int main(int UNUSED(args), char *argv[])
 	/*
 	 * Hooks that should run at the very end
 	 */
-	run_hooks(HOOK_SYSTEM_UP);
+	plugin_run_hooks(HOOK_SYSTEM_UP);
 
 	/* Start GETTY on console */
 	_d("Starting getty on console ...");

helpers.h

@@ -27,6 +27,7 @@
 
 #include <stdio.h>
 #include <syslog.h>
+#include <unistd.h>		/* access() */
 
 #define DO_LOG(level, fmt, args...)				\
 {								\
@@ -55,10 +56,10 @@
 #define UNUSED(x) UNUSED_ ## x __attribute__ ((unused))
 #endif
 
-#define echo(fmt, args...) do { if (1) { fprintf(stderr, fmt "\n",  ##args); } } while (0)
+#define echo(fmt, args...) do { if (verbose) { fprintf(stderr,                 fmt "\n", ##args); } } while (0)
-#define _d(fmt, args...)   do { if (debug)   { fprintf(stderr, "finit:%s() - " fmt "\n", __func__, ##args); } } while (0)
+#define   _d(fmt, args...) do { if (debug)   { fprintf(stderr, "finit:%s() - " fmt "\n", __func__, ##args); } } while (0)
-#define _e(fmt, args...)   do { fprintf(stderr, "finit:%s() - " fmt "\n", __func__, ##args); } while (0)
+#define   _e(fmt, args...) do {                fprintf(stderr, "finit:%s() - " fmt "\n", __func__, ##args); } while (0)
-#define _pe(fmt, args...)  do { fprintf(stderr, "finit:%s() - " fmt ". Error %d: %s\n", __func__, ##args, errno, strerror(errno)); } while (0)
+#define  _pe(fmt, args...) do {                fprintf(stderr, "finit:%s() - " fmt ". Error %d: %s\n", __func__, ##args, errno, strerror(errno)); } while (0)
 
 extern int   debug;
 extern int   verbose;

plugin.c

@@ -73,7 +73,6 @@ int plugin_register(plugin_t *plugin)
 
 		if (svc) {
 			plugin->svc.id = svc_id(svc);
-			svc->plugin    = &plugin->svc;
 			inuse++;
 		}
 	}
@@ -98,7 +97,7 @@ int plugin_unregister(plugin_t *plugin)
 }
 
 /* Private daemon API *******************************************************/
-void run_hooks(hook_point_t no)
+void plugin_run_hooks(hook_point_t no)
 {
 	plugin_t *p;
 
@@ -110,16 +109,19 @@ void run_hooks(hook_point_t no)
 	}
 }
 
-void run_services(void)
+/* Run registered plugin service callback for the given svc ID */
+svc_cmd_t plugin_svc_enabled(svc_t *svc, int event, void *arg)
 {
 	plugin_t *p;
 
+	/* Find matching plugin, pick first matching fd */
 	PLUGIN_ITERATOR(p) {
-		if (p->svc.cb) {
+		if (p->svc.id == svc_id(svc))
-			_d("Calling svc %s from runloop...", basename(p->name));
+			return p->svc.cb(svc, event, arg);
-			p->svc.cb(p->svc.arg, 0);
-		}
 	}
+
+	/* Unknown service, default to start (since it's in finit.conf) */
+	return SVC_START;
 }
 
 /* Generic libev I/O callback, looks up correct plugin and calls its callback */
@@ -137,7 +139,7 @@ static void generic_io_cb(int fd, int events)
 	}
 }
 
-void io_monitor(void)
+void plugin_monitor(void)
 {
 	int ret;
 	size_t i;
@@ -178,7 +180,7 @@ static void init_plugins(void)
 	}
 }
 
-int load_plugins(char *path)
+int plugin_load_all(char *path)
 {
 	DIR *dp = opendir(path);
 	struct dirent *entry;

plugin.h

@@ -26,6 +26,7 @@
 
 #include <poll.h>
 #include <sys/queue.h>		/* BSD sys/queue.h API */
+#include "svc.h"
 
 #define PLUGIN_IO_READ  POLLIN
 #define PLUGIN_IO_WRITE POLLOUT
@@ -65,7 +66,8 @@ typedef enum {
  * The "dynamic events" discussed in the svc callback is for external
  * service plugins to implement.  However, it can be anything that 
  * can cause a service to need to SIGHUP at runtime.  E.g., acquiring
- * a DHCP lease, or an interface going UP/DOWN.
+ * a DHCP lease, or an interface going UP/DOWN. The event itself can
+ * be passed as an integer @event and any optional argument @event_arg
  *
  * It is up to the external service plugin to track these events and
  * relay them to each @dynamic service plugins' callback.  I.e., to
@@ -80,14 +82,13 @@ typedef struct plugin {
 	char *name;
 
 	/* Service callback to be called once per lap of runloop. */
-	struct plugin_svc {
+	struct {
 		/* Private */
-		int    id;       /* Service ID# to match this service plugin against, set on installation. */
+		int         id;       /* Service ID# to match this service plugin against, set on installation. */
+
 		/* Public */
-		void  *arg;	 /* Optional argument to callback func, set by plugin. */
+		int         dynamic;  /* Reload (SIGHUP) on dynamic event?  Set by plugin. */
-		int    dynamic;	 /* Reload (SIGHUP) on dynamic event?  Set by plugin. */
+		svc_cmd_t (*cb)(svc_t *svc, int event, void *event_arg);
-		int    private;  /* For callbacks to use freely, possibly to store "states", set by plugin. */
-		int  (*cb)(void *arg, int event);
 	} svc;
 
 	/* List of hook callbacks. */
@@ -104,9 +105,6 @@ typedef struct plugin {
 	} io;
 } plugin_t;
 
-/* Used by svc.h */
-typedef struct plugin_svc plugin_svc_t;
-
 /* Public plugin API */
 int plugin_register   (plugin_t *plugin);
 int plugin_unregister (plugin_t *plugin);

private.h

@@ -25,13 +25,16 @@
 #ifndef FINIT_PRIVATE_H_
 #define FINIT_PRIVATE_H_
 
+#include "svc.h"
 #include "plugin.h"
 
-/* plugin.c */
+void      svc_start_all      (void);
-void run_hooks        (hook_point_t no);
+void      svc_monitor        (void);
-void run_services     (void);
+
-int  load_plugins     (char *path);
+void      plugin_run_hooks   (hook_point_t no);
-void io_monitor       (void);
+svc_cmd_t plugin_svc_enabled (svc_t *svc, int event, void *arg);
+int       plugin_load_all    (char *path);
+void      plugin_monitor     (void);
 
 #endif /* FINIT_PRIVATE_H_ */
 

sig.c

@@ -62,7 +62,7 @@ void do_shutdown (int sig)
 
 	sleep(1);
 	/* Call all shutdown hooks, this is the last call before rebooting... */
-	run_hooks(HOOK_SHUTDOWN);
+	plugin_run_hooks(HOOK_SHUTDOWN);
 
 	_d("Sending SIGKILL to remaining processes.");
 	kill(-1, SIGKILL);

svc.c

@@ -64,9 +64,9 @@ svc_t *svc_new(void)
 
 	__connect_shm();
 	if (svc_counter < MAX_NUM_SVC) {
-		svc = &services[svc_counter];
+		svc = &services[svc_counter++];
 		memset(svc, 0, sizeof(*svc));
-		svc->id = svc_counter++;
+		svc->id = svc_counter; /* Array pos + 1 to avoid zero ID */
 	} else {
 		errno = ENOMEM;
 		return NULL;
@@ -85,10 +85,10 @@ svc_t *svc_new(void)
 svc_t *svc_find_by_id(int id)
 {
 	__connect_shm();
-	if (id >= svc_counter)
+	if (id < 1 || id > svc_counter)
 		return NULL;
 
-	return &services[id];
+	return &services[id - 1];
 }
 
 /**
@@ -125,16 +125,16 @@ svc_t *svc_find_by_name(char *name)
  */
 svc_t *svc_iterator(int restart)
 {
-	static int id = 0;
+	static int i = 0;
 
 	__connect_shm();
 	if (restart)
-		id = 0;
+		i = 0;
 
-	if (id >= svc_counter)
+	if (i >= svc_counter)
 		return NULL;
 
-	return &services[id++];
+	return &services[i++];
 }
 
 /**
@@ -207,8 +207,9 @@ int svc_register(char *line, char *username)
 
 /**
  * svc_enabled - Should the service run?
- * @svc: Pointer to &svc_t object
+ * @svc:   Pointer to &svc_t object
- * @dynamic: Dynamic event, opaque flag passed to callback
+ * @event: Dynamic event, opaque flag passed to callback
+ * @arg:   Event argument, used only by external service plugins.
  *
  * This method calls an associated service callback, if registered by a
  * plugin, and returns the &svc_cmd_t status. If no plugin is registered
@@ -218,19 +219,14 @@ int svc_register(char *line, char *username)
  * Returns:
  * Either one of %SVC_START, %SVC_STOP, %SVC_RELOAD.
  */
-svc_cmd_t svc_enabled(svc_t *svc, int dynamic)
+svc_cmd_t svc_enabled(svc_t *svc, int event, void *arg)
 {
 	if (!svc) {
 		errno = EINVAL;
 		return SVC_STOP;
 	}
 
-	if (!svc->plugin) {
+	return plugin_svc_enabled(svc, event, arg);
-		/* Unknown service, default to enabled. */
-		return SVC_START;
-	}
-
-	return svc->plugin->cb(svc, dynamic);
 }
 
 static int is_norespawn(void)
@@ -273,7 +269,7 @@ void svc_monitor(void)
 		procname_kill(name, SIGTERM);
 
 		/* Restarting lost service. */
-		if (svc_enabled(svc, 0))
+		if (svc_enabled(svc, 0, NULL))
 			svc_start(svc);
 
 		break;
@@ -341,7 +337,7 @@ int svc_start_by_name(char *name)
 {
 	svc_t *svc = svc_find_by_name(name);
 
-	if (svc && svc_enabled(svc, 0))
+	if (svc && svc_enabled(svc, 0, NULL))
 		return svc_start(svc);
 
 	return 1;
@@ -394,7 +390,7 @@ void svc_start_all(void)
 	svc_cmd_t cmd;
 
 	for (svc = svc_iterator(1); svc; svc = svc_iterator(0)) {
-		cmd = svc_enabled(svc, 0);
+		cmd = svc_enabled(svc, 0, NULL);
 		if (cmd == SVC_START || (cmd == SVC_RELOAD && svc->pid == 0))
 			svc_start(svc);
 		else if (cmd == SVC_RELOAD)
@@ -402,7 +398,7 @@ void svc_start_all(void)
 	}
 
 	_d("Running svc up hooks ...");
-	run_hooks(HOOK_SVC_UP);
+	plugin_run_hooks(HOOK_SVC_UP);
 }
 
 

svc.h

@@ -30,8 +30,6 @@
 #include <sys/shm.h>		/* shmat() */
 #include <sys/types.h>		/* pid_t */
 
-#include "plugin.h"
-
 typedef enum {
 	SVC_STOP = 0,		/* Disabled */
 	SVC_START,		/* Enabled */
@@ -57,9 +55,10 @@ typedef struct svc {
 	char	       args[MAX_NUM_SVC_ARGS][MAX_ARG_LEN];
 	char	       desc[MAX_STR_LEN];
 	char           username[MAX_USER_LEN];
+
 	/* Public */
-	int	       reload; /* For external plugins. */
+	int	       reload;   /* For external plugins. */
-	plugin_svc_t  *plugin;
+	int            private;  /* For callbacks to use freely, possibly to store "states", set by plugin. */
 } svc_t;
 
 typedef struct svc_map svc_map_t;
@@ -96,13 +95,11 @@ svc_t    *svc_iterator      (int restart);
 
 int       svc_register      (char *line, char *username);
 int       svc_id_by_name    (char *name);
-svc_cmd_t svc_enabled	    (svc_t *svc, int dynamic);
+svc_cmd_t svc_enabled	    (svc_t *svc, int event, void *arg);
 int       svc_start         (svc_t *svc);
 int       svc_start_by_name (char *name);
 int       svc_stop          (svc_t *svc);
 int       svc_reload        (svc_t *svc);
-void      svc_start_all     (void);
-void      svc_monitor       (void);
 
 #endif	/* FINIT_SVC_H_ */