keventd: one condition emitter, one file reader, one clock

Three copies of read-first-line-from-a-file (sysfs_read_file, a
static sysfs_read, fgetline), two condition emitters symlinking the
same reconf marker (cond_emit, sys_cond), two seqnum-stable loops,
and five hand-rolled logit() prototypes, none with a printf
attribute, so no format string was ever checked.

Keep one of each: cond_emit() is exported and mkpaths its parents,
which also retires init_dev_condition_dir() and the sys/ directory
priming loop; logit() moves to keventd.h with the format attribute;
the stability test becomes seqnum_stable(), shared by the coldplug
gate and settle; kev_now_ms() serves dbus.c too.  rule_ctx_free()
and kev_seq_baseline() had one caller each in their own files, now
static.

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

keventd/builtin.c

@@ -41,7 +41,6 @@
 #include "sysfs.h"
 
 /* Forward declarations from keventd.c and uevent.c */
-void logit(int prio, const char *fmt, ...);
 int  modprobe_load(const char *modalias);
 
 /* Forward declarations — defined in the usb_id section below */

keventd/dbus.c

@@ -50,8 +50,6 @@
 #include "udevdb.h"
 #include "util.h"
 
-void logit(int prio, const char *fmt, ...);
-
 #define DEVBUS_DIR         "/run/keventd"
 #define DEVBUS_SOCKET      DEVBUS_DIR "/bus"
 #define DEVBUS_PATH_OBJECT "/org/finit/device"
@@ -86,14 +84,6 @@ static struct settle settles[DEVBUS_MAX_SETTLES];
 static uev_t         settle_timer;
 static int           settle_armed;
 
-static uint64_t now_ms(void)
-{
-	struct timespec ts;
-
-	clock_gettime(CLOCK_MONOTONIC, &ts);
-	return (uint64_t)ts.tv_sec * 1000 + ts.tv_nsec / 1000000;
-}
-
 /*
  * Same policy as /run/finit/bus: root and members of the --with-group
  * group.  The group set comes from the kernel (SO_PEERGROUPS), the
@@ -226,7 +216,7 @@ static void settle_sweep(uev_t *w, void *arg, int events);
  */
 static void settle_arm(uint64_t deadline)
 {
-	uint64_t now = now_ms();
+	uint64_t now = kev_now_ms();
 	int ms = deadline > now ? (int)(deadline - now) : 1;
 
 	if (!settle_armed) {
@@ -247,7 +237,7 @@ static void settle_arm(uint64_t deadline)
 static void settle_sweep(uev_t *w, void *arg, int events)
 {
 	uint64_t next = 0;
-	uint64_t now = now_ms();
+	uint64_t now = kev_now_ms();
 	int empty = kev_queue_empty();
 	int i;
 
@@ -305,7 +295,7 @@ static int device1_settle(link_call_t *call, void *u)
 				break;
 
 			settles[i].conn     = link_call_connection(call);
-			settles[i].deadline = now_ms() + (uint64_t)timeout * 1000;
+			settles[i].deadline = kev_now_ms() + (uint64_t)timeout * 1000;
 			settle_arm(settles[i].deadline);
 			return 0;
 		}

keventd/keventd.c

@@ -67,6 +67,7 @@
 
 #include "keventd.h"
 #include "rules.h"
+#include "sysfs.h"
 #include "udevdb.h"
 #include "cond.h"
 #include "pid.h"
@@ -81,10 +82,61 @@
 /* Used by settle and coldplug at startup */
 struct coldplug_gate {
 	unsigned long long last_seq;
-	struct timespec    last_change;
+	uint64_t           stamp;	/* kev_now_ms() of last change */
 	int                primed;
 };
 
+/*
+ * Read the kernel's uevent sequence counter.  Shared by the coldplug
+ * gate, settle mode, and the D-Bus queue state.
+ */
+static int read_uevent_seqnum(unsigned long long *out)
+{
+	FILE *fp;
+	int rc;
+
+	fp = fopen("/sys/kernel/uevent_seqnum", "r");
+	if (!fp)
+		return -1;
+
+	rc = fscanf(fp, "%llu", out) == 1 ? 0 : -1;
+	fclose(fp);
+
+	return rc;
+}
+
+uint64_t kev_now_ms(void)
+{
+	struct timespec ts;
+
+	clock_gettime(CLOCK_MONOTONIC, &ts);
+	return (uint64_t)ts.tv_sec * 1000 + ts.tv_nsec / 1000000;
+}
+
+/*
+ * Track the kernel seqnum: returns 1 once it has been unchanged for
+ * stable_ms, 0 while still moving (or before the first sample).
+ */
+static int seqnum_stable(struct coldplug_gate *cg, int stable_ms)
+{
+	unsigned long long cur = 0;
+	uint64_t now;
+
+	if (read_uevent_seqnum(&cur))
+		return 0;
+
+	now = kev_now_ms();
+
+	if (!cg->primed || cur != cg->last_seq) {
+		cg->last_seq = cur;
+		cg->stamp    = now;
+		cg->primed   = 1;
+		return 0;
+	}
+
+	return now - cg->stamp >= (uint64_t)stable_ms;
+}
+
 static int num_ac_online;
 static int num_ac;
 
@@ -181,39 +233,6 @@ static void rebc_event(char *buf, size_t len)
 		logit(LOG_DEBUG, "rebroadcast failed: %s", strerror(errno));
 }
 
-static void sys_cond(const char *cond, int set)
-{
-	char oneshot[256];
-
-	snprintf(oneshot, sizeof(oneshot), "%s/%s", _PATH_CONDSYS, cond);
-	if (set) {
-		if (symlink(_PATH_RECONF, oneshot) && errno != EEXIST)
-			warn("failed asserting sys/%s", cond);
-	} else {
-		if (erase(oneshot) && errno != ENOENT)
-			warn("failed asserting sys/%s", cond);
-	}
-}
-
-static int fgetline(const char *path, char *buf, size_t len)
-{
-	FILE *fp;
-
-	fp = fopen(path, "r");
-	if (!fp)
-		return -1;
-
-	if (!fgets(buf, len, fp)) {
-		fclose(fp);
-		return -1;
-	}
-
-	chomp(buf);
-	fclose(fp);
-
-	return 0;
-}
-
 static int check_online(const char *online)
 {
 	int val;
@@ -270,13 +289,13 @@ static void power_supply_change(const struct uevent *ev, char *buf, size_t len)
 		} else if (!strncmp(line, "POWER_SUPPLY_ONLINE=", 20) && ac) {
 			if (check_online(&line[20])) {
 				if (!num_ac_online)
-					sys_cond("pwr/ac", 1);
+					cond_emit(COND_SYS, "pwr/ac", 1);
 				num_ac_online++;
 			} else {
 				if (num_ac_online > 0)
 					num_ac_online--;
 				if (!num_ac_online)
-					sys_cond("pwr/ac", 0);
+					cond_emit(COND_SYS, "pwr/ac", 0);
 			}
 		}
 
@@ -429,32 +448,20 @@ static void handle_uevent(char *buf, size_t len)
 static void init_power_supply(void)
 {
 	struct dirent **d = NULL;
-	char *cond_dirs[] = {
-		_PATH_CONDSYS,
-		_PATH_CONDSYS "/pwr",
-	};
 	char path[384];
 	int i, n;
 
-	for (i = 0; i < (int)NELEMS(cond_dirs); i++) {
-		if (mkpath(cond_dirs[i], 0755) && errno != EEXIST) {
-			warn("Failed creating %s condition directory, %s", COND_SYS,
-			    cond_dirs[i]);
-			return;
-		}
-	}
-
 	n = scandir(_PATH_SYSFS_PWR, &d, NULL, alphasort);
 	for (i = 0; i < n; i++) {
 		const char *nm = d[i]->d_name;
 		char buf[10];
 
 		snprintf(path, sizeof(path), "%s/%s/type", _PATH_SYSFS_PWR, nm);
-		if (!fgetline(path, buf, sizeof(buf)) && is_ac(buf)) {
+		if (!sysfs_read_file(path, buf, sizeof(buf)) && is_ac(buf)) {
 			num_ac++;
 
 			snprintf(path, sizeof(path), "%s/%s/online", _PATH_SYSFS_PWR, nm);
-			if (!fgetline(path, buf, sizeof(buf))) {
+			if (!sysfs_read_file(path, buf, sizeof(buf))) {
 				if (check_online(buf))
 					num_ac_online++;
 			}
@@ -467,17 +474,7 @@ static void init_power_supply(void)
 
 	/* if any power_supply is online, or none can be found */
 	if (num_ac == 0 || num_ac_online > 0)
-		sys_cond("pwr/ac", 1);
-}
-
-static void init_dev_condition_dir(void)
-{
-	char dir[256];
-
-	/* Create /run/finit/cond/dev/ directory for device conditions */
-	snprintf(dir, sizeof(dir), "%s", _PATH_CONDDEV);
-	if (mkpath(dir, 0755) && errno != EEXIST)
-		warn("Failed creating dev condition directory %s", dir);
+		cond_emit(COND_SYS, "pwr/ac", 1);
 }
 
 static void disable_uevent_helper(void)
@@ -577,31 +574,12 @@ static void uevent_cb(uev_t *w, void *arg, int events)
  * means "/dev is populated, persistent symlinks are live" rather
  * than just "listening on netlink".
  */
-/*
- * Read the kernel's uevent sequence counter.  Shared by the coldplug
- * gate, settle mode, and the D-Bus queue state.
- */
-static int read_uevent_seqnum(unsigned long long *out)
-{
-	FILE *fp;
-	int rc;
-
-	fp = fopen("/sys/kernel/uevent_seqnum", "r");
-	if (!fp)
-		return -1;
-
-	rc = fscanf(fp, "%llu", out) == 1 ? 0 : -1;
-	fclose(fp);
-
-	return rc;
-}
-
 unsigned long long kev_seq_processed(void)
 {
 	return seq_processed;
 }
 
-void kev_seq_baseline(void)
+static void kev_seq_baseline(void)
 {
 	unsigned long long seq;
 
@@ -628,33 +606,10 @@ int kev_rules_reload(void)
 static void coldplug_pidfile_cb(uev_t *w, void *arg, int events)
 {
 	struct coldplug_gate *cg = arg;
-	unsigned long long cur = 0;
-	struct timespec now;
-	long dt_ms;
 
 	(void)events;
 
-	if (read_uevent_seqnum(&cur))
-		return;
-
-	clock_gettime(CLOCK_MONOTONIC, &now);
-
-	if (!cg->primed) {
-		cg->last_seq = cur;
-		cg->last_change = now;
-		cg->primed = 1;
-		return;
-	}
-
-	if (cur != cg->last_seq) {
-		cg->last_seq = cur;
-		cg->last_change = now;
-		return;
-	}
-
-	dt_ms = (now.tv_sec -  cg->last_change.tv_sec)  * 1000 +
-		(now.tv_nsec - cg->last_change.tv_nsec) / 1000000;
-	if (dt_ms < 200)
+	if (!seqnum_stable(cg, 200))
 		return;
 
 	pidfile(NULL);
@@ -672,32 +627,20 @@ static void coldplug_pidfile_cb(uev_t *w, void *arg, int events)
  */
 static int cmd_settle(int timeout_s, int stable_ms)
 {
-	unsigned long long last_seq = 0, cur_seq = 0;
-	struct timespec start, last_change, now;
-
-	clock_gettime(CLOCK_MONOTONIC, &start);
-	last_change = start;
+	struct coldplug_gate cg = { 0 };
+	uint64_t deadline = kev_now_ms() + (uint64_t)timeout_s * 1000;
 
 	while (1) {
-		if (read_uevent_seqnum(&cur_seq)) {
+		if (seqnum_stable(&cg, stable_ms))
+			return 0;
+
+		if (!cg.primed) {
 			fprintf(stderr, "keventd: cannot read uevent_seqnum: %s\n",
 				strerror(errno));
 			return 1;
 		}
 
-		clock_gettime(CLOCK_MONOTONIC, &now);
-
-		if (cur_seq != last_seq) {
-			last_seq = cur_seq;
-			last_change = now;
-		} else {
-			long dt_ms = (now.tv_sec - last_change.tv_sec) * 1000 +
-				     (now.tv_nsec - last_change.tv_nsec) / 1000000;
-			if (dt_ms >= stable_ms)
-				return 0;
-		}
-
-		if (now.tv_sec - start.tv_sec >= timeout_s)
+		if (kev_now_ms() >= deadline)
 			return 1;
 
 		usleep(50000);
@@ -828,7 +771,6 @@ int main(int argc, char *argv[])
 
 	/* Initialize condition directories */
 	init_power_supply();
-	init_dev_condition_dir();
 
 	/* Disable legacy kernel uevent helper; we own events via netlink.
 	 * Skip in passive mode -- the hotplug daemon handles this. */

keventd/keventd.h

@@ -135,7 +135,6 @@ struct dev_symlink {
 };
 
 /* Function prototypes - uevent.c */
-void            rule_ctx_free    (struct rule_ctx *ctx);
 
 int             uevent_parse     (char *buf, size_t len, struct uevent *ev);
 const char     *uevent_action_str(uevent_action_t action);
@@ -149,6 +148,7 @@ int             devnode_add      (struct uevent *ev);
 int             devnode_del      (struct uevent *ev);
 int             netdev_add       (struct uevent *ev);
 
+void            cond_emit        (const char *prefix, const char *rel, int set);
 void            class_cond       (const struct uevent *ev, int set);
 void            driver_cond      (const struct uevent *ev, int set);
 
@@ -163,9 +163,11 @@ int             coldplug         (void);
 int             coldplug_trigger (const char *action, const char *subsys_glob);
 int             uevent_action_valid(const char *action);
 
-/* keventd.c: queue and rules state, for the D-Bus layer */
+/* keventd.c: logging and queue/rules state, for all of keventd */
+void            logit            (int prio, const char *fmt, ...)
+	__attribute__((format(printf, 2, 3)));
+uint64_t        kev_now_ms       (void);
 unsigned long long kev_seq_processed(void);
-void            kev_seq_baseline (void);
 int             kev_queue_empty  (void);
 int             kev_rules_reload (void);
 

keventd/rules.c

@@ -51,9 +51,6 @@
 #include "sysfs.h"
 #include "udevdb.h"
 
-/* Forward declaration from keventd.c */
-void logit(int prio, const char *fmt, ...);
-
 /* ----- key-name lookup -------------------------------------------------- */
 
 static const struct {

keventd/udevdb.c

@@ -38,9 +38,6 @@
 #include "sysfs.h"
 #include "udevdb.h"
 
-/* Forward declaration from keventd.c */
-void logit(int prio, const char *fmt, ...);
-
 /*
  * Compute the database file path for a device.
  *
@@ -174,11 +171,6 @@ int udevdb_read(struct uevent *ev)
 	return udevdb_load(path, ev);
 }
 
-/*
- * Read the parent device's properties into ev's env store, for
- * IMPORT{parent} in the rules engine.  The parent has no uevent to key
- * from, so its device ID is composed from sysfs (see udevdb_path()).
- */
 /*
  * Read E: records for any devpath, keyed from its sysfs attributes:
  * the device has no uevent in hand, so major:minor, ifindex, and

keventd/uevent.c

@@ -55,9 +55,6 @@
 #include "sysfs.h"
 #include "util.h"
 
-/* Forward declarations */
-void logit(int prio, const char *fmt, ...);
-
 /*
  * Set/clear a Finit condition by creating/removing a symlink.
  * keventd is a standalone daemon, so we manipulate the filesystem directly
@@ -67,7 +64,7 @@ void logit(int prio, const char *fmt, ...);
  * COND_DRIVER, ...) ending in '/'.  rel is the relative path under that
  * namespace (e.g. "sda", "input/event0", "net/eth0", "mt7530").
  */
-static void cond_emit(const char *prefix, const char *rel, int set)
+void cond_emit(const char *prefix, const char *rel, int set)
 {
 	char cond[PATH_MAX];
 	char *dir;
@@ -437,7 +434,7 @@ int uevent_setenv(struct uevent *ev, const char *key, const char *val)
 	return 0;
 }
 
-void rule_ctx_free(struct rule_ctx *ctx)
+static void rule_ctx_free(struct rule_ctx *ctx)
 {
 	int i;
 
@@ -712,28 +709,6 @@ static int symlink_create(const char *target, const char *link, const char *devp
 	return 0;
 }
 
-/*
- * Read a single line from a sysfs attribute file.
- */
-static int sysfs_read(const char *path, char *buf, size_t len)
-{
-	FILE *fp;
-
-	fp = fopen(path, "r");
-	if (!fp)
-		return -1;
-
-	if (!fgets(buf, len, fp)) {
-		fclose(fp);
-		return -1;
-	}
-
-	fclose(fp);
-	chomp(buf);
-
-	return 0;
-}
-
 /*
  * Build disk ID string from sysfs attributes.
  * Format: BUSTYPE-VENDOR_MODEL_SERIAL
@@ -746,16 +721,16 @@ static int disk_serial_id(struct uevent *ev, char *id, size_t len)
 
 	/* Try to read from device's sysfs attributes */
 	snprintf(path, sizeof(path), "/sys%s/device/vendor", ev->devpath);
-	sysfs_read(path, vendor, sizeof(vendor));
+	sysfs_read_file(path, vendor, sizeof(vendor));
 
 	snprintf(path, sizeof(path), "/sys%s/device/model", ev->devpath);
-	sysfs_read(path, model, sizeof(model));
+	sysfs_read_file(path, model, sizeof(model));
 
 	snprintf(path, sizeof(path), "/sys%s/device/serial", ev->devpath);
-	if (sysfs_read(path, serial, sizeof(serial))) {
+	if (sysfs_read_file(path, serial, sizeof(serial))) {
 		/* Try alternate location */
 		snprintf(path, sizeof(path), "/sys%s/../serial", ev->devpath);
-		sysfs_read(path, serial, sizeof(serial));
+		sysfs_read_file(path, serial, sizeof(serial));
 	}
 
 	/* Need at least model or serial */
@@ -862,7 +837,7 @@ static int symlink_add_input(struct uevent *ev)
 
 	/* Read device name */
 	snprintf(path, sizeof(path), "/sys%s/device/name", ev->devpath);
-	if (sysfs_read(path, name, sizeof(name)))
+	if (sysfs_read_file(path, name, sizeof(name)))
 		return -1;
 
 	/* Clean up name */
@@ -878,7 +853,7 @@ static int symlink_add_input(struct uevent *ev)
 
 	/* by-path: physical path (if available) */
 	snprintf(path, sizeof(path), "/sys%s/device/phys", ev->devpath);
-	if (!sysfs_read(path, phys, sizeof(phys))) {
+	if (!sysfs_read_file(path, phys, sizeof(phys))) {
 		for (p = phys; *p; p++) {
 			if (*p == ' ' || *p == '/')
 				*p = '_';