Re: [PATCH] Make saved environment available for remove event rules

[Kay Sievers <kay.sievers@vrfy.org>]

[-- Attachment #1: Type: text/plain, Size: 1546 bytes --]

On Wed, Apr 05, 2006 at 09:51:06PM +0400, Andrey Borzenkov wrote:
> The patch assures that saved environment is available when processing
> rules for 'remove' event. It enables mathing on previously set variables.
> Among others it makes REMOVE_CMD really works (Debian bug 360855).
> 
> This is slightly modified version of patch from this thread:
> http://marc.theaimsgroup.com/?l=linux-hotplug-devel&m=114416867706787&w=2
> 
> Signed-off-by: Andrey Borzenkov <arvidjaar@mail.ru>
> 
> ---
> Patch attached due to kmail customarily mangling long lines :(
> 
> -andrey

> Subject: [PATCH] Make saved environment available for remove event rules
> 
> From: Andrey Borzenkov <arvidjaar@mail.ru>
> 
> The patch assures that saved environment is available when processing
> rules for 'remove' event. It enables mathing on previously set variables.
> Among others it makes REMOVE_CMD really works (Debian bug 360855).
> 
> This is slightly modified version of patch from this thread:
> http://marc.theaimsgroup.com/?l=linux-hotplug-devel&m=114416867706787&w=2

This looks fine, but I already played around with merging the node
handling code into one file and moving the database calls out of the
node functions.
It's a preparation for a future extension to be able to store data
for devices that don't create a node.

Attached is the pretty big patch (looks smaller in git) and it's not
really tested. I changed the call to the db like your patch is doing.
Would be nice if you can have a look if that is correct and would work
for you.

Thanks,
Kay

[-- Attachment #2: udev-unify-event-02.patch --]
[-- Type: text/plain, Size: 18202 bytes --]

diff --git a/Makefile b/Makefile
index 1ce1abe..42c2bf2 100644
--- a/Makefile
+++ b/Makefile
@@ -69,8 +69,7 @@ HEADERS = \
 UDEV_OBJS = \
 	udev_device.o			\
 	udev_config.o			\
-	udev_add.o			\
-	udev_remove.o			\
+	udev_node.o			\
 	udev_db.o			\
 	udev_sysfs.o			\
 	udev_rules.o			\
diff --git a/udev.h b/udev.h
index 2652d46..615253a 100644
--- a/udev.h
+++ b/udev.h
@@ -117,12 +117,10 @@ extern struct sysfs_device *sysfs_device
 extern struct sysfs_device *sysfs_device_get_parent_with_subsystem(struct sysfs_device *dev, const char *subsystem);
 extern char *sysfs_attr_get_value(const char *devpath, const char *attr_name);
 
-/* udev_add.c */
-extern int udev_make_node(struct udevice *udev, const char *file, dev_t devt, mode_t mode, uid_t uid, gid_t gid);
-extern int udev_add_device(struct udevice *udev);
-
-/* udev_remove.c */
-extern int udev_remove_device(struct udevice *udev);
+/* udev_node.c */
+extern int udev_node_mknod(struct udevice *udev, const char *file, dev_t devt, mode_t mode, uid_t uid, gid_t gid);
+extern int udev_node_add(struct udevice *udev);
+extern int udev_node_remove(struct udevice *udev);
 
 /* udev_db.c */
 extern int udev_db_add_device(struct udevice *dev);
diff --git a/udev_device.c b/udev_device.c
index 5db2709..14c8d39 100644
--- a/udev_device.c
+++ b/udev_device.c
@@ -1,7 +1,7 @@
 /*
- * udev_utils.c - generic stuff used by udev
+ * udev_device.c - main udev data object
  *
- * Copyright (C) 2004, 2005 Kay Sievers <kay.sievers@vrfy.org>
+ * Copyright (C) 2004-2006 Kay Sievers <kay.sievers@vrfy.org>
  *
  *	This program is free software; you can redistribute it and/or modify it
  *	under the terms of the GNU General Public License as published by the
@@ -26,6 +26,9 @@
 #include <fcntl.h>
 #include <errno.h>
 #include <ctype.h>
+#include <stropts.h>
+#include <net/if.h>
+#include <linux/sockios.h>
 
 #include "udev.h"
 #include "udev_rules.h"
@@ -77,48 +80,122 @@ dev_t udev_device_get_devt(struct udevic
 	return makedev(0, 0);
 }
 
+static int rename_net_if(struct udevice *udev)
+{
+	int sk;
+	struct ifreq ifr;
+	int retval;
+
+	info("changing net interface name from '%s' to '%s'", udev->dev->kernel_name, udev->name);
+	if (udev->test_run)
+		return 0;
+
+	sk = socket(PF_INET, SOCK_DGRAM, 0);
+	if (sk < 0) {
+		err("error opening socket: %s", strerror(errno));
+		return -1;
+	}
+
+	memset(&ifr, 0x00, sizeof(struct ifreq));
+	strlcpy(ifr.ifr_name, udev->dev->kernel_name, IFNAMSIZ);
+	strlcpy(ifr.ifr_newname, udev->name, IFNAMSIZ);
+
+	retval = ioctl(sk, SIOCSIFNAME, &ifr);
+	if (retval != 0)
+		err("error changing net interface name: %s", strerror(errno));
+	close(sk);
+
+	return retval;
+}
+
 int udev_device_event(struct udev_rules *rules, struct udevice *udev)
 {
 	int retval = 0;
 
-	/* device node or netif */
-	if ((major(udev->devt) != 0 || strcmp(udev->dev->subsystem, "net") == 0) &&
-	    strcmp(udev->action, "add") == 0) {
-		dbg("device node or netif add '%s'", udev->dev->devpath);
+	/* add device node */
+	if (major(udev->devt) != 0 && strcmp(udev->action, "add") == 0) {
+		dbg("device node add '%s'", udev->dev->devpath);
 		udev_rules_get_name(rules, udev);
 		if (udev->ignore_device) {
 			info("device event will be ignored");
-			return 0;
+			goto exit;
 		}
-		/* create node, store in db */
-		if (udev->name[0] != '\0')
-			retval = udev_add_device(udev);
-		else
+		if (udev->name[0] == '\0') {
 			info("device node creation supressed");
-		return retval;
+			goto exit;
+		}
+		/* create node, store in db */
+		retval = udev_node_add(udev);
+		if (retval == 0)
+			udev_db_add_device(udev);
+		goto exit;
 	}
 
+	/* add netif */
+	if (strcmp(udev->dev->subsystem, "net") == 0 && strcmp(udev->action, "add") == 0) {
+		dbg("netif add '%s'", udev->dev->devpath);
+		udev_rules_get_name(rules, udev);
+		if (udev->ignore_device) {
+			info("device event will be ignored");
+			goto exit;
+		}
+
+		/* look if we want to change the name of the netif */
+		if (strcmp(udev->name, udev->dev->kernel_name) != 0) {
+			char *pos;
+
+			retval = rename_net_if(udev);
+			if (retval != 0)
+				goto exit;
+			info("renamed netif to '%s'", udev->name);
+
+			/* now fake the devpath, because the kernel name changed silently */
+			pos = strrchr(udev->dev->devpath, '/');
+			if (pos != NULL) {
+				pos[1] = '\0';
+				strlcat(udev->dev->devpath, udev->name, sizeof(udev->dev->devpath));
+				strlcpy(udev->dev->kernel_name, udev->name, sizeof(udev->dev->kernel_name));
+				setenv("DEVPATH", udev->dev->devpath, 1);
+				setenv("INTERFACE", udev->name, 1);
+			}
+		}
+		goto exit;
+	}
+
+	/* remove device node */
 	if (major(udev->devt) != 0 && strcmp(udev->action, "remove") == 0) {
 		struct name_entry *name_loop;
 
+		/* import and delete database entry */
+		if (udev_db_get_device(udev, udev->dev->devpath) == 0) {
+			udev_db_delete_device(udev);
+			if (udev->ignore_remove) {
+				dbg("remove event for '%s' requested to be ignored by rule", udev->name);
+				return 0;
+			}
+			/* restore stored persistent data */
+			list_for_each_entry(name_loop, &udev->env_list, node)
+				putenv(name_loop->name);
+		} else {
+			dbg("'%s' not found in database, using kernel name '%s'", udev->dev->devpath, udev->dev->kernel_name);
+			strlcpy(udev->name, udev->dev->kernel_name, sizeof(udev->name));
+		}
+
 		udev_rules_get_run(rules, udev);
 		if (udev->ignore_device) {
 			info("device event will be ignored");
-			return 0;
+			goto exit;
 		}
-		/* get data from db, remove db-entry, delete node */
-		retval = udev_remove_device(udev);
 
-		/* restore stored persistent data */
-		list_for_each_entry(name_loop, &udev->env_list, node)
-			putenv(name_loop->name);
-		return retval;
+		retval = udev_node_remove(udev);
+		goto exit;
 	}
 
-	/* default devices without a node */
+	/* default devices */
 	udev_rules_get_run(rules, udev);
 	if (udev->ignore_device)
 		info("device event will be ignored");
 
+exit:
 	return retval;
 }
diff --git a/udev_add.c b/udev_node.c
similarity index 67%
rename from udev_add.c
rename to udev_node.c
index 6c023ab..9d6e89c 100644
--- a/udev_add.c
+++ b/udev_node.c
@@ -1,5 +1,5 @@
 /*
- * udev-add.c
+ * udev-node.c - device node handling
  *
  * Copyright (C) 2003 Greg Kroah-Hartman <greg@kroah.com>
  * Copyright (C) 2004 Kay Sievers <kay.sievers@vrfy.org>
@@ -29,17 +29,13 @@
 #include <grp.h>
 #include <sys/stat.h>
 #include <sys/types.h>
-#include <sys/socket.h>
-#include <sys/ioctl.h>
-#include <net/if.h>
-#include <linux/sockios.h>
 
 #include "udev.h"
 #include "udev_rules.h"
 #include "udev_selinux.h"
 
 
-int udev_make_node(struct udevice *udev, const char *file, dev_t devt, mode_t mode, uid_t uid, gid_t gid)
+int udev_node_mknod(struct udevice *udev, const char *file, dev_t devt, mode_t mode, uid_t uid, gid_t gid)
 {
 	struct stat stats;
 	int retval = 0;
@@ -94,7 +90,7 @@ exit:
 	return retval;
 }
 
-static int create_node(struct udevice *udev)
+int udev_node_add(struct udevice *udev)
 {
 	char filename[PATH_SIZE];
 	struct name_entry *name_loop;
@@ -102,6 +98,9 @@ static int create_node(struct udevice *u
 	gid_t gid;
 	int tail;
 	int i;
+	int retval = 0;
+
+	selinux_init();
 
 	snprintf(filename, sizeof(filename), "%s/%s", udev_root, udev->name);
 	filename[sizeof(filename)-1] = '\0';
@@ -140,8 +139,10 @@ static int create_node(struct udevice *u
 	     filename, major(udev->devt), minor(udev->devt), udev->mode, uid, gid);
 
 	if (!udev->test_run)
-		if (udev_make_node(udev, filename, udev->devt, udev->mode, uid, gid) != 0)
-			goto error;
+		if (udev_node_mknod(udev, filename, udev->devt, udev->mode, uid, gid) != 0) {
+			retval = -1;
+			goto exit;
+		}
 
 	setenv("DEVNAME", filename, 1);
 
@@ -166,7 +167,7 @@ static int create_node(struct udevice *u
 				snprintf(partitionname, sizeof(partitionname), "%s%d", filename, i);
 				partitionname[sizeof(partitionname)-1] = '\0';
 				part_devt = makedev(major(udev->devt), minor(udev->devt) + i);
-				udev_make_node(udev, partitionname, part_devt, udev->mode, uid, gid);
+				udev_node_mknod(udev, partitionname, part_devt, udev->mode, uid, gid);
 			}
 		}
 	}
@@ -176,7 +177,6 @@ static int create_node(struct udevice *u
 		char symlinks[512] = "";
 
 		list_for_each_entry(name_loop, &udev->symlink_list, node) {
-			int retval;
 			char linktarget[PATH_SIZE];
 
 			snprintf(filename, sizeof(filename), "%s/%s", udev_root, name_loop->name);
@@ -208,11 +208,9 @@ static int create_node(struct udevice *u
 			if (!udev->test_run) {
 				unlink(filename);
 				selinux_setfscreatecon(filename, NULL, S_IFLNK);
-				retval = symlink(linktarget, filename);
+				if (symlink(linktarget, filename) != 0)
+					err("symlink(%s, %s) failed: %s", linktarget, filename, strerror(errno));
 				selinux_resetfscreatecon();
-				if (retval != 0)
-					err("symlink(%s, %s) failed: %s",
-					    linktarget, filename, strerror(errno));
 			}
 
 			strlcat(symlinks, filename, sizeof(symlinks));
@@ -223,78 +221,87 @@ static int create_node(struct udevice *u
 		setenv("DEVLINKS", symlinks, 1);
 	}
 
-	return 0;
-error:
-	return -1;
+exit:
+	selinux_exit();
+	return retval;
 }
 
-static int rename_net_if(struct udevice *udev)
+int udev_node_remove(struct udevice *udev)
 {
-	int sk;
-	struct ifreq ifr;
+	char filename[PATH_SIZE];
+	char partitionname[PATH_SIZE];
+	struct name_entry *name_loop;
+	struct stat stats;
 	int retval;
+	int i;
+	int num;
 
-	info("changing net interface name from '%s' to '%s'", udev->dev->kernel_name, udev->name);
-	if (udev->test_run)
-		return 0;
-
-	sk = socket(PF_INET, SOCK_DGRAM, 0);
-	if (sk < 0) {
-		err("error opening socket: %s", strerror(errno));
-		return -1;
-	}
+	if (!list_empty(&udev->symlink_list)) {
+		char symlinks[512] = "";
 
-	memset(&ifr, 0x00, sizeof(struct ifreq));
-	strlcpy(ifr.ifr_name, udev->dev->kernel_name, IFNAMSIZ);
-	strlcpy(ifr.ifr_newname, udev->name, IFNAMSIZ);
-
-	retval = ioctl(sk, SIOCSIFNAME, &ifr);
-	if (retval != 0)
-		err("error changing net interface name: %s", strerror(errno));
-	close(sk);
+		list_for_each_entry(name_loop, &udev->symlink_list, node) {
+			snprintf(filename, sizeof(filename), "%s/%s", udev_root, name_loop->name);
+			filename[sizeof(filename)-1] = '\0';
 
-	return retval;
-}
+			if (stat(filename, &stats) != 0) {
+				dbg("symlink '%s' not found", filename);
+				continue;
+			}
+			if (udev->devt && stats.st_rdev != udev->devt) {
+				info("symlink '%s' points to a different device, skip removal", filename);
+				continue;
+			}
 
-int udev_add_device(struct udevice *udev)
-{
-	char *pos;
-	int retval = 0;
+			info("removing symlink '%s'", filename);
+			unlink(filename);
 
-	dbg("adding name='%s'", udev->name);
-	selinux_init();
+			if (strchr(filename, '/'))
+				delete_path(filename);
 
-	if (major(udev->devt)) {
-		retval = create_node(udev);
-		if (retval != 0)
-			goto exit;
+			strlcat(symlinks, filename, sizeof(symlinks));
+			strlcat(symlinks, " ", sizeof(symlinks));
+		}
 
-		if (udev_db_add_device(udev) != 0)
-			dbg("udev_db_add_dev failed, remove might not work for custom names");
-	} else if (strcmp(udev->dev->subsystem, "net") == 0) {
-		/* look if we want to change the name of the netif */
-		if (strcmp(udev->name, udev->dev->kernel_name) != 0) {
-			retval = rename_net_if(udev);
-			if (retval != 0)
-				goto exit;
-
-			info("renamed netif to '%s'", udev->name);
-			/* we've changed the name, now fake the devpath, because the
-			 * original kernel name sleeps with the fishes and we don't
-			 * get an event from the kernel with the new name
-			 */
-			pos = strrchr(udev->dev->devpath, '/');
-			if (pos != NULL) {
-				pos[1] = '\0';
-				strlcat(udev->dev->devpath, udev->name, sizeof(udev->dev->devpath));
-				strlcpy(udev->dev->kernel_name, udev->name, sizeof(udev->dev->kernel_name));
-				setenv("DEVPATH", udev->dev->devpath, 1);
-				setenv("INTERFACE", udev->name, 1);
-			}
+		remove_trailing_chars(symlinks, ' ');
+		if (symlinks[0] != '\0')
+			setenv("DEVLINKS", symlinks, 1);
+	}
+
+	snprintf(filename, sizeof(filename), "%s/%s", udev_root, udev->name);
+	filename[sizeof(filename)-1] = '\0';
+
+	if (stat(filename, &stats) != 0) {
+		dbg("device node '%s' not found", filename);
+		return -1;
+	}
+	if (udev->devt && stats.st_rdev != udev->devt) {
+		info("device node '%s' points to a different device, skip removal", filename);
+		return -1;
+	}
+
+	info("removing device node '%s'", filename);
+	retval = unlink_secure(filename);
+	if (retval)
+		return retval;
+
+	setenv("DEVNAME", filename, 1);
+
+	num = udev->partitions;
+	if (num > 0) {
+		info("removing all_partitions '%s[1-%i]'", filename, num);
+		if (num > 255) {
+			info("garbage from udev database, skip all_partitions removal");
+			return -1;
+		}
+		for (i = 1; i <= num; i++) {
+			snprintf(partitionname, sizeof(partitionname), "%s%d", filename, i);
+			partitionname[sizeof(partitionname)-1] = '\0';
+			unlink_secure(partitionname);
 		}
 	}
 
-exit:
-	selinux_exit();
+	if (strchr(udev->name, '/'))
+		delete_path(filename);
+
 	return retval;
 }
diff --git a/udev_remove.c b/udev_remove.c
deleted file mode 100644
index 6de6cf5..0000000
--- a/udev_remove.c
+++ /dev/null
@@ -1,135 +0,0 @@
-/*
- * udev-remove.c
- *
- * Copyright (C) 2003 Greg Kroah-Hartman <greg@kroah.com>
- * Copyright (C) 2004 Kay Sievers <kay.sievers@vrfy.org>
- *
- *	This program is free software; you can redistribute it and/or modify it
- *	under the terms of the GNU General Public License as published by the
- *	Free Software Foundation version 2 of the License.
- * 
- *	This program is distributed in the hope that it will be useful, but
- *	WITHOUT ANY WARRANTY; without even the implied warranty of
- *	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- *	General Public License for more details.
- * 
- *	You should have received a copy of the GNU General Public License along
- *	with this program; if not, write to the Free Software Foundation, Inc.,
- *	675 Mass Ave, Cambridge, MA 02139, USA.
- *
- */
-
-#include <stdlib.h>
-#include <string.h>
-#include <stddef.h>
-#include <stdio.h>
-#include <fcntl.h>
-#include <unistd.h>
-#include <errno.h>
-#include <sys/stat.h>
-
-#include "udev.h"
-
-static int delete_node(struct udevice *udev)
-{
-	char filename[PATH_SIZE];
-	char partitionname[PATH_SIZE];
-	struct name_entry *name_loop;
-	struct stat stats;
-	int retval;
-	int i;
-	int num;
-
-	if (!list_empty(&udev->symlink_list)) {
-		char symlinks[512] = "";
-
-		list_for_each_entry(name_loop, &udev->symlink_list, node) {
-			snprintf(filename, sizeof(filename), "%s/%s", udev_root, name_loop->name);
-			filename[sizeof(filename)-1] = '\0';
-
-			if (stat(filename, &stats) != 0) {
-				dbg("symlink '%s' not found", filename);
-				continue;
-			}
-			if (udev->devt && stats.st_rdev != udev->devt) {
-				info("symlink '%s' points to a different device, skip removal", filename);
-				continue;
-			}
-
-			info("removing symlink '%s'", filename);
-			unlink(filename);
-
-			if (strchr(filename, '/'))
-				delete_path(filename);
-
-			strlcat(symlinks, filename, sizeof(symlinks));
-			strlcat(symlinks, " ", sizeof(symlinks));
-		}
-
-		remove_trailing_chars(symlinks, ' ');
-		if (symlinks[0] != '\0')
-			setenv("DEVLINKS", symlinks, 1);
-	}
-
-	snprintf(filename, sizeof(filename), "%s/%s", udev_root, udev->name);
-	filename[sizeof(filename)-1] = '\0';
-
-	if (stat(filename, &stats) != 0) {
-		dbg("device node '%s' not found", filename);
-		return -1;
-	}
-	if (udev->devt && stats.st_rdev != udev->devt) {
-		info("device node '%s' points to a different device, skip removal", filename);
-		return -1;
-	}
-
-	info("removing device node '%s'", filename);
-	retval = unlink_secure(filename);
-	if (retval)
-		return retval;
-
-	setenv("DEVNAME", filename, 1);
-
-	num = udev->partitions;
-	if (num > 0) {
-		info("removing all_partitions '%s[1-%i]'", filename, num);
-		if (num > 255) {
-			info("garbage from udev database, skip all_partitions removal");
-			return -1;
-		}
-		for (i = 1; i <= num; i++) {
-			snprintf(partitionname, sizeof(partitionname), "%s%d", filename, i);
-			partitionname[sizeof(partitionname)-1] = '\0';
-			unlink_secure(partitionname);
-		}
-	}
-
-	if (strchr(udev->name, '/'))
-		delete_path(filename);
-
-	return retval;
-}
-
-/*
- * look up the sysfs path in the database to get the node name to remove
- * If we can't find it, use kernel name for lack of anything else to know to do
- */
-int udev_remove_device(struct udevice *udev)
-{
-	if (major(udev->devt) == 0)
-		return 0;
-
-	if (udev_db_get_device(udev, udev->dev->devpath) == 0) {
-		if (udev->ignore_remove) {
-			dbg("remove event for '%s' requested to be ignored by rule", udev->name);
-			return 0;
-		}
-		dbg("remove name='%s'", udev->name);
-		udev_db_delete_device(udev);
-	} else {
-		dbg("'%s' not found in database, using kernel name '%s'", udev->dev->devpath, udev->dev->kernel_name);
-		strlcpy(udev->name, udev->dev->kernel_name, sizeof(udev->name));
-	}
-
-	return delete_node(udev);
-}
diff --git a/udev_rules.c b/udev_rules.c
index 1aedb11..09a299f 100644
--- a/udev_rules.c
+++ b/udev_rules.c
@@ -610,7 +610,7 @@ found:
 				snprintf(udev->tmp_node, sizeof(udev->tmp_node), "%s/.tmp-%u-%u",
 					 udev_root, major(udev->devt), minor(udev->devt));
 				udev->tmp_node[sizeof(udev->tmp_node)-1] = '\0';
-				udev_make_node(udev, udev->tmp_node, udev->devt, 0600, 0, 0);
+				udev_node_mknod(udev, udev->tmp_node, udev->devt, 0600, 0, 0);
 			}
 			strlcat(string, udev->tmp_node, maxsize);
 			dbg("substitute temporary device node name '%s'", udev->tmp_node);
diff --git a/udevstart.c b/udevstart.c
index e5ce7af..0d9b4f4 100644
--- a/udevstart.c
+++ b/udevstart.c
@@ -130,7 +130,6 @@ static int add_device(const char *devpat
 	/* override built-in sysfs device */
 	udev->dev = dev;
 	strcpy(udev->action, "add");
-	udev->devt = udev_device_get_devt(udev);
 
 	if (strcmp(udev->dev->subsystem, "net") != 0) {
 		udev->devt = udev_device_get_devt(udev);
@@ -148,7 +147,7 @@ static int add_device(const char *devpat
 		goto exit;
 	}
 	if (udev->name[0] != '\0')
-		retval = udev_add_device(udev);
+		retval = udev_device_event(&rules, udev);
 	else
 		info("device node creation supressed");