udevd: simplify udev database and fix DEVNAME handling

4 files changed, 87 insertions, 136 deletions

diff --git a/libudev/libudev-device-private.c b/libudev/libudev-device-private.c
index 63f947e16b..bf93834c10 100644
--- a/libudev/libudev-device-private.c
+++ b/libudev/libudev-device-private.c
@@ -72,96 +72,83 @@ int udev_device_tag_index(struct udev_device *dev, struct udev_device *dev_old,
 	return 0;
 }
 
+static bool device_has_info(struct udev_device *udev_device)
+{
+	struct udev_list_entry *list_entry;
+
+	if (udev_device_get_devlinks_list_entry(udev_device) != NULL)
+		return true;
+	if (udev_device_get_devlink_priority(udev_device) != 0)
+		return true;
+	udev_list_entry_foreach(list_entry, udev_device_get_properties_list_entry(udev_device))
+		if (udev_list_entry_get_flags(list_entry))
+			return true;
+	if (udev_device_get_tags_list_entry(udev_device) != NULL)
+		return true;
+	if (udev_device_get_watch_handle(udev_device) >= 0)
+		return true;
+	return false;
+}
+
 int udev_device_update_db(struct udev_device *udev_device)
 {
+	bool has_info;
 	const char *id;
 	struct udev *udev = udev_device_get_udev(udev_device);
 	char filename[UTIL_PATH_SIZE];
 	char filename_tmp[UTIL_PATH_SIZE];
 	FILE *f;
-	char target[232]; /* on 64bit, tmpfs inlines up to 239 bytes */
 	size_t devlen = strlen(udev_get_dev_path(udev))+1;
-	char *s;
-	size_t l;
-	struct udev_list_entry *list_entry;
-	int ret;
 
 	id = udev_device_get_id_filename(udev_device);
 	if (id == NULL)
 		return -1;
+
+	has_info = device_has_info(udev_device);
 	util_strscpyl(filename, sizeof(filename), udev_get_dev_path(udev), "/.udev/db/", id, NULL);
-	util_strscpyl(filename_tmp, sizeof(filename_tmp), filename, ".tmp", NULL);
 
-	udev_list_entry_foreach(list_entry, udev_device_get_properties_list_entry(udev_device))
-		if (udev_list_entry_get_flags(list_entry))
-			goto file;
-	if (udev_device_get_tags_list_entry(udev_device) != NULL)
-		goto file;
-	if (udev_device_get_devlink_priority(udev_device) != 0)
-		goto file;
-	if (udev_device_get_event_timeout(udev_device) >= 0)
-		goto file;
-	if (udev_device_get_watch_handle(udev_device) >= 0)
-		goto file;
-	if (udev_device_get_devnode(udev_device) == NULL)
-		goto out;
-
-	/*
-	 * if we have only the node and symlinks to store, try not to waste
-	 * tmpfs memory -- store values, if they fit, in a symlink target
-	 */
-	s = target;
-	l = util_strpcpy(&s, sizeof(target), &udev_device_get_devnode(udev_device)[devlen]);
-	udev_list_entry_foreach(list_entry, udev_device_get_devlinks_list_entry(udev_device)) {
-		l = util_strpcpyl(&s, l, " ", &udev_list_entry_get_name(list_entry)[devlen], NULL);
-		if (l == 0) {
-			info(udev, "size of links too large, create file\n");
-			goto file;
-		}
+	/* do not store anything for otherwise empty devices */
+	if (!has_info && udev_device_get_devnode(udev_device) == NULL) {
+		unlink(filename);
+		return 0;
 	}
-	udev_selinux_setfscreatecon(udev, filename_tmp, S_IFLNK);
-	util_create_path(udev, filename_tmp);
-	ret = symlink(target, filename_tmp);
-	udev_selinux_resetfscreatecon(udev);
-	if (ret != 0)
-		goto file;
-	ret = rename(filename_tmp, filename);
-	if (ret != 0)
-		goto file;
-	info(udev, "created db link (%s)\n", target);
-	goto out;
-file:
+
+	/* write a database file */
+	util_strscpyl(filename_tmp, sizeof(filename_tmp), filename, ".tmp", NULL);
 	util_create_path(udev, filename_tmp);
-	f = fopen(filename_tmp, "w");
+	f = fopen(filename_tmp, "we");
 	if (f == NULL) {
 		err(udev, "unable to create temporary db file '%s': %m\n", filename_tmp);
 		return -1;
 	}
 
-	if (udev_device_get_devnode(udev_device) != NULL) {
-		fprintf(f, "N:%s\n", &udev_device_get_devnode(udev_device)[devlen]);
-		udev_list_entry_foreach(list_entry, udev_device_get_devlinks_list_entry(udev_device))
-			fprintf(f, "S:%s\n", &udev_list_entry_get_name(list_entry)[devlen]);
-	}
-	if (udev_device_get_devlink_priority(udev_device) != 0)
-		fprintf(f, "L:%i\n", udev_device_get_devlink_priority(udev_device));
-	if (udev_device_get_event_timeout(udev_device) >= 0)
-		fprintf(f, "T:%i\n", udev_device_get_event_timeout(udev_device));
-	if (udev_device_get_watch_handle(udev_device) >= 0)
-		fprintf(f, "W:%i\n", udev_device_get_watch_handle(udev_device));
-	udev_list_entry_foreach(list_entry, udev_device_get_properties_list_entry(udev_device)) {
-		if (!udev_list_entry_get_flags(list_entry))
-			continue;
-		fprintf(f, "E:%s=%s\n",
-			udev_list_entry_get_name(list_entry),
-			udev_list_entry_get_value(list_entry));
+	if (has_info) {
+		struct udev_list_entry *list_entry;
+
+		if (udev_device_get_devnode(udev_device) != NULL) {
+			fprintf(f, "N:%s\n", &udev_device_get_devnode(udev_device)[devlen]);
+			udev_list_entry_foreach(list_entry, udev_device_get_devlinks_list_entry(udev_device))
+				fprintf(f, "S:%s\n", &udev_list_entry_get_name(list_entry)[devlen]);
+		}
+		if (udev_device_get_devlink_priority(udev_device) != 0)
+			fprintf(f, "L:%i\n", udev_device_get_devlink_priority(udev_device));
+		if (udev_device_get_watch_handle(udev_device) >= 0)
+			fprintf(f, "W:%i\n", udev_device_get_watch_handle(udev_device));
+		udev_list_entry_foreach(list_entry, udev_device_get_properties_list_entry(udev_device)) {
+			if (!udev_list_entry_get_flags(list_entry))
+				continue;
+			fprintf(f, "E:%s=%s\n",
+				udev_list_entry_get_name(list_entry),
+				udev_list_entry_get_value(list_entry));
+		}
+		udev_list_entry_foreach(list_entry, udev_device_get_tags_list_entry(udev_device))
+			fprintf(f, "G:%s\n", udev_list_entry_get_name(list_entry));
 	}
-	udev_list_entry_foreach(list_entry, udev_device_get_tags_list_entry(udev_device))
-		fprintf(f, "G:%s\n", udev_list_entry_get_name(list_entry));
+
 	fclose(f);
 	rename(filename_tmp, filename);
-	info(udev, "created db file for '%s' in '%s'\n", udev_device_get_devpath(udev_device), filename);
-out:
+	info(udev, "created %s file '%s' for '%s'\n", has_info ? "db" : "empty", 
+	     filename, udev_device_get_devpath(udev_device));
 	return 0;
 }
 
diff --git a/libudev/libudev-device.c b/libudev/libudev-device.c
index d87b0c6c8a..ac206a0e16 100644
--- a/libudev/libudev-device.c
+++ b/libudev/libudev-device.c
@@ -232,70 +232,28 @@ const char *udev_device_get_property_value(struct udev_device *udev_device, cons
 int udev_device_read_db(struct udev_device *udev_device)
 {
 	const char *id;
-	struct stat stats;
 	char filename[UTIL_PATH_SIZE];
 	char line[UTIL_LINE_SIZE];
 	FILE *f;
 
 	if (udev_device->db_loaded)
 		return 0;
+	udev_device->db_loaded = true;
 
 	id = udev_device_get_id_filename(udev_device);
 	if (id == NULL)
 		return -1;
 	util_strscpyl(filename, sizeof(filename), udev_get_dev_path(udev_device->udev), "/.udev/db/", id, NULL);
-
-	if (lstat(filename, &stats) != 0) {
-		dbg(udev_device->udev, "no db file to read %s: %m\n", filename);
-		return -1;
-	}
-	if ((stats.st_mode & S_IFMT) == S_IFLNK) {
-		char target[UTIL_PATH_SIZE];
-		char devnode[UTIL_PATH_SIZE];
-		int target_len;
-		char *next;
-
-		target_len = readlink(filename, target, sizeof(target));
-		if (target_len <= 0 || target_len == sizeof(target)) {
-			info(udev_device->udev, "error reading db link %s: %m\n", filename);
-			return -1;
-		}
-		target[target_len] = '\0';
-
-		next = strchr(target, ' ');
-		if (next != NULL) {
-			next[0] = '\0';
-			next = &next[1];
-		}
-		util_strscpyl(devnode, sizeof(devnode), udev_get_dev_path(udev_device->udev), "/", target, NULL);
-		udev_device_set_devnode(udev_device, devnode);
-		while (next != NULL) {
-			char devlink[UTIL_PATH_SIZE];
-			const char *lnk;
-
-			lnk = next;
-			next = strchr(next, ' ');
-			if (next != NULL) {
-				next[0] = '\0';
-				next = &next[1];
-			}
-			util_strscpyl(devlink, sizeof(devlink), udev_get_dev_path(udev_device->udev), "/", lnk, NULL);
-			udev_device_add_devlink(udev_device, devlink, 0);
-		}
-		info(udev_device->udev, "device %p filled with db symlink data '%s'\n", udev_device, udev_device->devnode);
-		return 0;
-	}
-
 	f = fopen(filename, "re");
 	if (f == NULL) {
-		dbg(udev_device->udev, "error reading db file %s: %m\n", filename);
+		info(udev_device->udev, "no db file to read %s: %m\n", filename);
 		return -1;
 	}
-	udev_device->db_loaded = true;
 
 	while (fgets(line, sizeof(line), f)) {
 		ssize_t len;
 		const char *val;
+		struct udev_list_entry *entry;
 
 		len = strlen(line);
 		if (len < 4)
@@ -314,11 +272,9 @@ int udev_device_read_db(struct udev_device *udev_device)
 		case 'L':
 			udev_device_set_devlink_priority(udev_device, atoi(val));
 			break;
-		case 'T':
-			udev_device_set_event_timeout(udev_device, atoi(val));
-			break;
 		case 'E':
-			udev_device_add_property_from_string(udev_device, val);
+			entry = udev_device_add_property_from_string(udev_device, val);
+			udev_list_entry_set_flags(entry, 1);
 			break;
 		case 'G':
 			udev_device_add_tag(udev_device, val);
@@ -888,9 +844,11 @@ const char *udev_device_get_devnode(struct udev_device *udev_device)
 
 	/* we might get called before we handled an event and have a db, use the kernel-provided name */
 	if (udev_device->devnode == NULL && udev_device_get_knodename(udev_device) != NULL) {
-		if (asprintf(&udev_device->devnode, "%s/%s",
-			     udev_get_dev_path(udev_device->udev), udev_device_get_knodename(udev_device)) < 0)
-			return NULL;
+		char filename[UTIL_NAME_SIZE];
+
+		util_strscpyl(filename, sizeof(filename), udev_get_dev_path(udev_device->udev), "/",
+			      udev_device_get_knodename(udev_device), NULL);
+		udev_device_set_devnode(udev_device, filename);
 		return udev_device->devnode;
 	}
 
@@ -1271,8 +1229,6 @@ int udev_device_set_devnode(struct udev_device *udev_device, const char *devnode
 {
 	free(udev_device->devnode);
 	udev_device->devnode = strdup(devnode);
-	if (devnode == NULL)
-		return 0;
 	if (udev_device->devnode == NULL)
 		return -ENOMEM;
 	udev_device_add_property(udev_device, "DEVNAME", udev_device->devnode);
@@ -1516,7 +1472,9 @@ int udev_device_set_knodename(struct udev_device *udev_device, const char *knode
 	udev_device->knodename = strdup(knodename);
 	if (udev_device->knodename == NULL)
 		return -ENOMEM;
-	udev_device_add_property(udev_device, "DEVNAME", udev_device->knodename);
+	/* do not overwrite the udev property with the kernel property */
+	if (udev_device->devnode == NULL)
+		udev_device_add_property(udev_device, "DEVNAME", udev_device->knodename);
 	return 0;
 }
 
diff --git a/udev/udev-node.c b/udev/udev-node.c
index 70488c420b..a9ffa988b1 100644
--- a/udev/udev-node.c
+++ b/udev/udev-node.c
@@ -327,7 +327,7 @@ static void link_update(struct udev_device *dev, const char *slink, bool add)
 			err = util_create_path(udev, filename);
 			if (err != 0 && err != -ENOENT)
 				break;
-			fd = open(filename, O_WRONLY|O_CREAT|O_NOFOLLOW, 0444);
+			fd = open(filename, O_WRONLY|O_CREAT|O_CLOEXEC|O_TRUNC|O_NOFOLLOW, 0444);
 			if (fd >= 0)
 				close(fd);
 			else
diff --git a/udev/udevadm-info.c b/udev/udevadm-info.c
index 7206f4fcf6..4510f4aa90 100644
--- a/udev/udevadm-info.c
+++ b/udev/udevadm-info.c
@@ -210,38 +210,44 @@ static int convert_db(struct udev *udev)
 		device = udev_device_new_from_syspath(udev, udev_list_entry_get_name(list_entry));
 		if (device != NULL) {
 			const char *id;
-			struct stat statbuf;
+			struct stat stats;
 			char to[UTIL_PATH_SIZE];
 			char devpath[UTIL_PATH_SIZE];
 			char from[UTIL_PATH_SIZE];
 
 			id = udev_device_get_id_filename(device);
-			if (id == NULL)
-				goto next;
+			if (id == NULL) {
+				udev_device_unref(device);
+				continue;
+			}
 			util_strscpyl(to, sizeof(to), udev_get_dev_path(udev), "/.udev/db/", id, NULL);
 
-			/* do not overwrite a new database file */
-			if (lstat(to, &statbuf) == 0)
-				goto next;
-
 			/* find old database with $subsys:$sysname */
 			util_strscpyl(from, sizeof(from), udev_get_dev_path(udev),
 				     "/.udev/db/", udev_device_get_subsystem(device), ":",
 				     udev_device_get_sysname(device), NULL);
-			if (lstat(from, &statbuf) == 0) {
-				rename(from, to);
-				goto next;
+			if (lstat(from, &stats) == 0) {
+				if (lstat(to, &stats) == 0)
+					unlink(from);
+				else
+					rename(from, to);
 			}
 
 			/* find old database with the encoded devpath */
 			util_path_encode(udev_device_get_devpath(device), devpath, sizeof(devpath));
 			util_strscpyl(from, sizeof(from), udev_get_dev_path(udev),
 				      "/.udev/db/", devpath, NULL);
-			if (lstat(from, &statbuf) == 0) {
-				rename(from, to);
-				goto next;
+			if (lstat(from, &stats) == 0) {
+				if (lstat(to, &stats) == 0)
+					unlink(from);
+				else
+					rename(from, to);
 			}
-next:
+
+			/* read the old database, and write out a new one */
+			udev_device_read_db(device);
+			udev_device_update_db(device);
+
 			udev_device_unref(device);
 		}
 	}