[PATCH] cleanup mult field string handling

Here I try to cleanup our various multifield iteration over the strings.
Inspired by our nice list.h we now have a macro to iterate over the string
and process the parts of it:
It makes the code more readable and we don't change the string while we
process it like the former strsep() does.

Example:

  foreach_strpart(dev->symlink, " ", pos, len) {
  	if (strncmp(&dev->symlink[pos], find_name, len) != 0)
  		continue;

  	...
  }

For the callout part selector %c{2} we separate now not only by space but
also newline and return characters, cause some programs may give multiline
values back. A possible RESULT match must contain wildcards for these
characters.

Also a bug in the recent udevinfo symlink query feature is fixed.

namedev.c

@@ -214,14 +214,12 @@ static void apply_format(struct udevice *udev, char *string, size_t maxsize,
 			 struct sysfs_device *sysfs_device)
 {
 	char temp[NAME_SIZE];
-	char temp1[NAME_SIZE];
 	char *tail;
 	char *pos;
-	char *pos2;
-	char *pos3;
 	char *attr;
 	int len;
 	int i;
+	int spos, slen;
 	char c;
 	struct sysfs_attribute *tmpattr;
 
@@ -278,20 +276,17 @@ static void apply_format(struct udevice *udev, char *string, size_t maxsize,
 			if (attr != NULL)
 				i = atoi(attr);
 			if (i > 0) {
-				strfieldcpy(temp1, udev->program_result);
-				pos2 = temp1;
-				while (i) {
+				foreach_strpart(udev->program_result, " \n\r", spos, slen) {
 					i--;
-					pos3 = strsep(&pos2, " ");
-					if (pos3 == NULL) {
-						dbg("requested part of result string not found");
+					if (i == 0)
 						break;
-					}
 				}
-				if (pos3) {
-					strnfieldcat(string, pos3, maxsize);
-					dbg("substitute part of result string '%s'", pos3);
+				if (i > 0) {
+					dbg("requested part of result string not found");
+					break;
 				}
+				strnfieldcat(string, udev->program_result + spos, slen+1);
+				dbg("substitute part of result string '%s'", pos);
 			} else {
 				strnfieldcat(string, udev->program_result, maxsize);
 				dbg("substitute result string '%s'", udev->program_result);

udev-add.c

@@ -186,16 +186,16 @@ static int unlink_entry(char *filename)
 
 static int create_node(struct udevice *dev, int fake)
 {
-	char filename[255];
-	char linktarget[255];
-	char partitionname[255];
-	char *linkname;
-	char *symlinks;
+	char filename[NAME_SIZE];
+	char linkname[NAME_SIZE];
+	char linktarget[NAME_SIZE];
+	char partitionname[NAME_SIZE];
 	int retval = 0;
 	uid_t uid = 0;
 	gid_t gid = 0;
 	int i;
 	int tail;
+	int pos, len;
 
 	strfieldcpy(filename, udev_root);
 	strfieldcat(filename, dev->name);
@@ -279,47 +279,41 @@ static int create_node(struct udevice *dev, int fake)
 		selinux_add_node(filename);
 
 	/* create symlink if requested */
-	if (dev->symlink[0] != '\0') {
-		symlinks = dev->symlink;
-		while (1) {
-			linkname = strsep(&symlinks, " ");
-			if (linkname == NULL || linkname[0] == '\0')
-				break;
+	foreach_strpart(dev->symlink, " ", pos, len) {
+		strnfieldcpy(linkname, dev->symlink + pos, len+1);
+		strfieldcpy(filename, udev_root);
+		strfieldcat(filename, linkname);
+		dbg("symlink '%s' to node '%s' requested", filename, dev->name);
+		if (!fake)
+			if (strrchr(linkname, '/'))
+				create_path(filename);
 
-			strfieldcpy(filename, udev_root);
-			strfieldcat(filename, linkname);
-			dbg("symlink '%s' to node '%s' requested", filename, dev->name);
-			if (!fake)
-				if (strrchr(linkname, '/'))
-					create_path(filename);
+		/* optimize relative link */
+		linktarget[0] = '\0';
+		i = 0;
+		tail = 0;
+		while ((dev->name[i] == linkname[i]) && dev->name[i]) {
+			if (dev->name[i] == '/')
+				tail = i+1;
+			i++;
+		}
+		while (linkname[i] != '\0') {
+			if (linkname[i] == '/')
+				strfieldcat(linktarget, "../");
+			i++;
+		}
 
-			/* optimize relative link */
-			linktarget[0] = '\0';
-			i = 0;
-			tail = 0;
-			while ((dev->name[i] == linkname[i]) && dev->name[i]) {
-				if (dev->name[i] == '/')
-					tail = i+1;
-				i++;
-			}
-			while (linkname[i] != '\0') {
-				if (linkname[i] == '/')
-					strfieldcat(linktarget, "../");
-				i++;
-			}
+		strfieldcat(linktarget, &dev->name[tail]);
 
-			strfieldcat(linktarget, &dev->name[tail]);
+		if (!fake)
+			unlink_entry(filename);
 
-			if (!fake)
-				unlink_entry(filename);
-
-			dbg("symlink(%s, %s)", linktarget, filename);
-			if (!fake) {
-				retval = symlink(linktarget, filename);
-				if (retval != 0)
-					dbg("symlink(%s, %s) failed with error '%s'",
-					    linktarget, filename, strerror(errno));
-			}
+		dbg("symlink(%s, %s)", linktarget, filename);
+		if (!fake) {
+			retval = symlink(linktarget, filename);
+			if (retval != 0)
+				dbg("symlink(%s, %s) failed with error '%s'",
+				    linktarget, filename, strerror(errno));
 		}
 	}
 

udev-remove.c

@@ -67,12 +67,12 @@ static int delete_path(char *path)
 
 static int delete_node(struct udevice *dev)
 {
-	char filename[255];
-	char partitionname[255];
-	char *symlinks;
-	char *linkname;
+	char filename[NAME_SIZE];
+	char linkname[NAME_SIZE];
+	char partitionname[NAME_SIZE];
 	int retval;
 	int i;
+	int pos, len;
 
 	strfieldcpy(filename, udev_root);
 	strfieldcat(filename, dev->name);
@@ -101,28 +101,22 @@ static int delete_node(struct udevice *dev)
 	if (strchr(dev->name, '/'))
 		delete_path(filename);
 
-	if (dev->symlink[0] != '\0') {
-		symlinks = dev->symlink;
-		while (1) {
-			linkname = strsep(&symlinks, " ");
-			if (linkname == NULL)
-				break;
+	foreach_strpart(dev->symlink, " ", pos, len) {
+		strnfieldcpy(linkname, dev->symlink + pos, len+1);
+		strfieldcpy(filename, udev_root);
+		strfieldcat(filename, linkname);
 
-			strfieldcpy(filename, udev_root);
-			strfieldcat(filename, linkname);
-
-			dbg("unlinking symlink '%s'", filename);
-			retval = unlink(filename);
-			if (errno == ENOENT)
-				retval = 0;
-			if (retval) {
-				dbg("unlink(%s) failed with error '%s'",
-					filename, strerror(errno));
-				return retval;
-			}
-			if (strchr(dev->symlink, '/')) {
-				delete_path(filename);
-			}
+		dbg("unlinking symlink '%s'", filename);
+		retval = unlink(filename);
+		if (errno == ENOENT)
+			retval = 0;
+		if (retval) {
+			dbg("unlink(%s) failed with error '%s'",
+				filename, strerror(errno));
+			return retval;
+		}
+		if (strchr(dev->symlink, '/')) {
+			delete_path(filename);
 		}
 	}
 

udev.h

@@ -97,6 +97,12 @@ do { \
 	snprintf((to) + strlen(to), maxsize - strlen(to)-1, "%u", i); \
 } while (0)
 
+#define foreach_strpart(str, separator, pos, len) \
+	for(pos = 0, len = strcspn(str, separator); \
+	    (pos) < strlen(str); \
+	    pos = pos + (len) + 1, len = strcspn((str) + pos, separator)) \
+		if (len > 0)
+
 static inline char *get_action(void)
 {
 	char *action;

udevdb.c

@@ -179,7 +179,8 @@ static int find_found;
 
 static int find_device_by_name(char *path, struct udevice *dev)
 {
-	int l, i, j;
+	int pos, len;
+
 	if (strncmp(dev->name, find_name, sizeof(dev->name)) == 0) {
 		memcpy(find_dev, dev, sizeof(struct udevice));
 		strnfieldcpy(find_path, path, NAME_SIZE);
@@ -188,20 +189,18 @@ static int find_device_by_name(char *path, struct udevice *dev)
 		return 1;
 	}
 	/* look for matching symlink*/
-	l = strlen(dev->symlink);
-	if (!l)
-		return 0;
-	i = j = 0;
-	do {
-		j = strcspn(&dev->symlink[i], " ");
-		if (j && strncmp(&dev->symlink[i], find_name, j) == 0) {
-			memcpy(find_dev, dev, sizeof(struct udevice));
-			strnfieldcpy(find_path, path, NAME_SIZE);
-			find_found = 1;
-			return 1;
-		}
-		i = i + j + 1;
-	} while (i < l);
+	foreach_strpart(dev->symlink, " ", pos, len) {
+		if (strncmp(&dev->symlink[pos], find_name, len) != 0)
+			continue;
+
+		if (len != strlen(find_name))
+			continue;
+
+		memcpy(find_dev, dev, sizeof(struct udevice));
+		strnfieldcpy(find_path, path, NAME_SIZE);
+		find_found = 1;
+		return 1;
+	}
 	return 0;
 }
 

udevinfo.8

@@ -35,6 +35,8 @@ Specify the sysfs path of the device to query.
 .TP
 .BI \-n " name"
 Specify the name of the node or the symlink for the device to query.
+Partition names generated with the NAME{all_partitons} option can not be
+queried, the main device must be used instead.
 .TP
 .B \-a
 Print all