[PATCH 3/4] mdev: Fix -i
Isaac Dunham
ibid.ag at gmail.com
Mon Apr 13 17:15:34 UTC 2015
* strcmp() logic was inverted.
* Keep a default path.
We don't use a dynamically allocated buffer since clearenv() *could*
free that.
* Avoid a slow leak in the reader, whereby we copy strings, add them to
the environment, clear the environment, and forget it.
* micro-optimizations:
- char *value was used only once; inline it
- use key rather than recalculating msgbuf + i.
* Multiple hotplug events can arrive in the pipe before read() is called,
so we could end up getting a single handle_event() call for multiple
hotplug events.
So, the proposed protocol is to use a second '\0' at the end of each
hotplug event.
* Make mdev -i optional
* mdev -i and -s both are run from userspace
* free msgbuf if needed
* remove stale #define
* don't assume that if argv[1] is present, it's an option
Documentation/usb/hotplug.txt states that argv[1] may be "usb".
---
util-linux/mdev.c | 111 +++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 84 insertions(+), 27 deletions(-)
diff --git a/util-linux/mdev.c b/util-linux/mdev.c
index 689fe3a..0088305 100644
--- a/util-linux/mdev.c
+++ b/util-linux/mdev.c
@@ -55,6 +55,15 @@
//config:
//config: For more information, please see docs/mdev.txt
//config:
+//config:config FEATURE_MDEV_PIPE
+//config: bool "Support reading hotplug events from pipe"
+//config: default y
+//config: depends on MDEV
+//config: help
+//config: This adds support for "mdev -i", letting mdev read events
+//config: from stdin. An event is submitted as a series of
+//config: NUL-terminated strings, followed by a NUL byte.
+//config:
//config:config FEATURE_MDEV_LOAD_FIRMWARE
//config: bool "Support loading of firmwares"
//config: default y
@@ -71,11 +80,13 @@
//kbuild:lib-$(CONFIG_MDEV) += mdev.o
//usage:#define mdev_trivial_usage
-//usage: "[-s|-i]"
+//usage: IF_FEATURE_MDEV_PIPE("[-s|-i]")
+//usage: IF_NOT_FEATURE_MDEV_PIPE("[-s]")
//usage:#define mdev_full_usage "\n\n"
//usage: "mdev -s is to be run during boot to scan /sys and populate /dev.\n"
-//usage: "\n"
+//usage: IF_FEATURE_MDEV_PIPE(
//usage: "mdev -i will read events from stdin\n"
+//usage: )
//usage: "\n"
//usage: "Bare mdev is a kernel hotplug helper. To activate it:\n"
//usage: " echo /sbin/mdev >/proc/sys/kernel/hotplug\n"
@@ -300,6 +311,16 @@ struct globals {
/* We use additional bytes in make_device() */
#define SCRATCH_SIZE 128
+/* Where are the first two bytes marking the end of a message?
+ * (ie, first "\0\0" in msgbuf)
+ */
+static size_t msg_end(char *msgbuf, size_t bufsiz)
+{
+ size_t i = 0;
+ while ((i<bufsiz) && (msgbuf[i] || msgbuf[i+1])) i++;
+ return i;
+}
+
#if ENABLE_FEATURE_MDEV_CONF
static void make_default_cur_rule(void)
@@ -825,6 +846,7 @@ static int FAST_FUNC fileAction(const char *fileName,
strcpy(scratch, fileName);
scratch[len] = '\0';
+ // XXX: Make this use the uevent file?
make_device(/*DEVNAME:*/ NULL, scratch, OP_add);
return TRUE;
@@ -1015,7 +1037,12 @@ static void signal_mdevs(unsigned my_pid)
}
}
-static void handle_event(char *temp, int usage_on_error)
+/* XXX: When we move to general key-based lookup (getkey instead of getenv),
+ * this should become (char *temp, char *buf, size_t bufsiz)
+ * and !bufsiz can be used as usage_on_error.
+ * Eventually, this path should be used for mdev -s as well.
+ */
+static void handle_event(char *temp, char *buf, size_t bsiz)
{
char *fw;
char *seq;
@@ -1026,6 +1053,20 @@ static void handle_event(char *temp, int usage_on_error)
int seq_fd;
smalluint op;
+ if (bsiz) {
+ size_t i, slen = 0;
+
+ putenv((char*)bb_PATH_root_path);
+ for (i = 0; i < bsiz; i += slen + 1) {
+ char *key = buf + i;
+
+ slen = strlen(key);
+ if (!slen || strchr(key, '=') == NULL)
+ continue;
+ putenv(key);
+ }
+ }
+
/* Hotplug:
* env ACTION=... DEVPATH=... SUBSYSTEM=... [SEQNUM=...] mdev
* ACTION can be "add", "remove", "change"
@@ -1036,7 +1077,7 @@ static void handle_event(char *temp, int usage_on_error)
action = getenv("ACTION");
env_devpath = getenv("DEVPATH");
if (!action || !env_devpath /*|| !G.subsystem*/) {
- if (usage_on_error)
+ if (!bsiz)
bb_show_usage();
return;
}
@@ -1073,6 +1114,9 @@ static void handle_event(char *temp, int usage_on_error)
}
}
+ if (bsiz)
+ clearenv();
+
dbg1("%s exiting", curtime());
if (seq_fd >= 0) {
xwrite_str(seq_fd, utoa(xatou(seq) + 1));
@@ -1093,7 +1137,7 @@ int mdev_main(int argc UNUSED_PARAM, char **argv)
/* We can be called as hotplug helper */
/* Kernel cannot provide suitable stdio fds for us, do it ourself */
- if (argv[1] && strcmp(argv[1], "-i") != 0)
+ IF_FEATURE_MDEV_PIPE(if (!argv[1] || argv[1][0] != '-'))
bb_sanitize_stdio();
/* Force the configuration file settings exactly */
@@ -1136,41 +1180,54 @@ int mdev_main(int argc UNUSED_PARAM, char **argv)
recursive_action("/sys/class",
ACTION_RECURSE | ACTION_FOLLOWLINKS,
fileAction, dirAction, temp, 0);
- } else if (argv[1] && strcmp(argv[1], "-i") != 0) {
- RESERVE_CONFIG_BUFFER(msgbuf, 16*1024);
+ } else if (ENABLE_FEATURE_MDEV_PIPE && argv[1] && strcmp(argv[1], "-i") == 0) {
+#define MSGBUFSIZE 16*1024
+ RESERVE_CONFIG_BUFFER(msgbuf, MSGBUFSIZE);
struct pollfd fds;
- int r;
+ int r, len = 0;
fds.fd = 0;
fds.events = POLLIN;
- while ((r = poll(&fds, 1, 2000)) > 0) {
- int i, len, slen;
- clearenv();
- if (!(fds.revents & POLLIN))
- continue;
- len = read(fds.fd, msgbuf, sizeof(msgbuf));
- for (i = 0; i < len; i += slen + 1) {
- char *key, *value;
+ while (((r = poll(&fds, 1, 2000)) > 0) ||
+ (msg_end(msgbuf, MSGBUFSIZE) + 1 < len)) {
+ int i, nlen;
- key = msgbuf + i;
- value = strchr(key, '=');
- slen = strlen(msgbuf+i);
- if (!slen || value == NULL)
- continue;
+ if (fds.revents & POLLIN) {
+ nlen = read(fds.fd, msgbuf+len, MSGBUFSIZE-len);
+ if (nlen > 1)
+ len += nlen;
+ }
- value[0] = '\0';
- value++;
+ if (len < msg_end(msgbuf, MSGBUFSIZE)) {
+ continue;
+ }
+
+ handle_event(temp, msgbuf, len);
+ i = msg_end(msgbuf, len) + 1;
- setenv(key, value, 1);
+ /* if i < len, we have at least part of one more
+ * message, and need to move it up and save it.
+ * We will have at least one full event if it
+ * was written atomically, since an event is
+ * under 4096 bytes.
+ */
+ if ((i < len)) {
+ i++;
+ len = len - i;
+ memmove(msgbuf, msgbuf + i, len);
+ } else {
+ len = 0;
}
- handle_event(temp, 0);
- if (fds.revents & POLLHUP)
+ if ((fds.revents & POLLHUP) && (len - 1 < 0 ||
+ (msg_end(msgbuf, MSGBUFSIZE) > len - 1)))
break;
}
+ if (ENABLE_FEATURE_CLEAN_UP)
+ RELEASE_CONFIG_BUFFER(msgbuf);
if (r == -1)
return EXIT_FAILURE;
} else {
- handle_event(temp, 1);
+ handle_event(temp, NULL, 0);
}
if (ENABLE_FEATURE_CLEAN_UP)
--
2.1.4
More information about the busybox
mailing list