[PATCH RFC] Merge udev_scan_block() and udev_scan_class() in udevstart.c

[PATCH RFC] Merge udev_scan_block() and udev_scan_class() in udevstart.c

[Tobias Klauser]

udev_scan_block() and udev_scan_class() in udevstart.c have a lot of
redundant code and therefor I was thinking about a way to merge them.
My patch might not be the most elegant solution but it reduces the code
about 40 lines (see diffstat).

Comments, Flames etc are welcome.

Cheers, Tobias


 udevstart.c |   71 ++++++++++++------------------------------------------------
 1 files changed, 15 insertions(+), 56 deletions(-)

--- udev-058/udevstart.c	2005-05-20 22:18:28.000000000 +0200
+++ udev-058~tk/udevstart.c	2005-06-05 13:44:28.218251600 +0200
@@ -204,14 +204,14 @@ static int has_devt(const char *director
 	return 0;
 }
 
-static void udev_scan_block(void)
+static void udev_scan_dir(char *type)
 {
 	char base[PATH_SIZE];
 	DIR *dir;
 	struct dirent *dent;
 	LIST_HEAD(device_list);
 
-	snprintf(base, sizeof(base), "%s/block", sysfs_path);
+	snprintf(base, sizeof(base), "%s/%s", sysfs_path, type);
 	base[sizeof(base)-1] = '\0';
 
 	dir = opendir(base);
@@ -227,56 +227,10 @@ static void udev_scan_block(void)
 			snprintf(dirname, sizeof(dirname), "%s/%s", base, dent->d_name);
 			dirname[sizeof(dirname)-1] = '\0';
 			if (has_devt(dirname))
-				device_list_insert(dirname, "block", &device_list);
+				device_list_insert(dirname, type, &device_list);
 			else
 				continue;
 
-			/* look for partitions */
-			dir2 = opendir(dirname);
-			if (dir2 != NULL) {
-				for (dent2 = readdir(dir2); dent2 != NULL; dent2 = readdir(dir2)) {
-					char dirname2[PATH_SIZE];
-
-					if (dent2->d_name[0] = '.')
-						continue;
-
-					snprintf(dirname2, sizeof(dirname2), "%s/%s", dirname, dent2->d_name);
-					dirname2[sizeof(dirname2)-1] = '\0';
-
-					if (has_devt(dirname2))
-						device_list_insert(dirname2, "block", &device_list);
-				}
-				closedir(dir2);
-			}
-		}
-		closedir(dir);
-	}
-	exec_list(&device_list);
-}
-
-static void udev_scan_class(void)
-{
-	char base[PATH_SIZE];
-	DIR *dir;
-	struct dirent *dent;
-	LIST_HEAD(device_list);
-
-	snprintf(base, sizeof(base), "%s/class", sysfs_path);
-	base[sizeof(base)-1] = '\0';
-
-	dir = opendir(base);
-	if (dir != NULL) {
-		for (dent = readdir(dir); dent != NULL; dent = readdir(dir)) {
-			char dirname[PATH_SIZE];
-			DIR *dir2;
-			struct dirent *dent2;
-
-			if (dent->d_name[0] = '.')
-				continue;
-
-			snprintf(dirname, sizeof(dirname), "%s/%s", base, dent->d_name);
-			dirname[sizeof(dirname)-1] = '\0';
-
 			dir2 = opendir(dirname);
 			if (dir2 != NULL) {
 				for (dent2 = readdir(dir2); dent2 != NULL; dent2 = readdir(dir2)) {
@@ -288,11 +242,16 @@ static void udev_scan_class(void)
 					snprintf(dirname2, sizeof(dirname2), "%s/%s", dirname, dent2->d_name);
 					dirname2[sizeof(dirname2)-1] = '\0';
 
-					/* pass the net class as it is */
-					if (strcmp(dent->d_name, "net") = 0)
-						device_list_insert(dirname2, "net", &device_list);
-					else if (has_devt(dirname2))
-						device_list_insert(dirname2, dent->d_name, &device_list);
+					if (strncmp(type, "block", 5) = 0) {
+						if (has_devt(dirname2))
+							device_list_insert(dirname2, type, &device_list);
+					} else if (strncmp(type, "class", 5) = 0) {
+						/* pass the net class as it is */
+						if (strcmp(dent->d_name, "net") = 0)
+							device_list_insert(dirname2, "net", &device_list);
+						else if (has_devt(dirname2))
+							device_list_insert(dirname2, dent->d_name, &device_list);
+					}
 				}
 				closedir(dir2);
 			}
@@ -342,8 +301,8 @@ int main(int argc, char *argv[], char *e
 
 	udev_rules_init();
 
-	udev_scan_block();
-	udev_scan_class();
+	udev_scan_dir("block");
+	udev_scan_dir("class");
 
 	logging_close();
 	return 0;


-------------------------------------------------------
This SF.Net email is sponsored by: NEC IT Guy Games.  How far can you shotput
a projector? How fast can you ride your desk chair down the office luge track?
If you want to score the big prize, get to know the little guy.  
Play to win an NEC 61" plasma display: http://www.necitguy.com/?r 
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH RFC] Merge udev_scan_block() and udev_scan_class() in udevstart.c

[Kay Sievers]

On Sun, Jun 05, 2005 at 03:00:16PM +0200, Tobias Klauser wrote:
> udev_scan_block() and udev_scan_class() in udevstart.c have a lot of
> redundant code and therefor I was thinking about a way to merge them.
> My patch might not be the most elegant solution but it reduces the code
> about 40 lines (see diffstat).

You will not get any class device with that patch! This prevents going
down into the directories where the "dev" files are, it works only
for block devices, cause they find a "dev" at the parent, but
class-directories are empty here:

  if (has_devt(dirname))
  	device_list_insert(dirname, type, &device_list);
  else
  	continue;

Please test more carefully before changing that bootup-critical parts of
udev. The test directory contains a udevstart-test for that reason.

There is a reason for these functions to be separate, but if you still think
that this is worth to do, you may check the dirent type:
  DTYPE(de) = DT_DIR
instead of "dev" file lookups at every entry, which happens more often after
merging these too functions.
Anyway, I'm not convinced, that these functions are not better kept separate.

Kay


-------------------------------------------------------
This SF.Net email is sponsored by: NEC IT Guy Games.  How far can you shotput
a projector? How fast can you ride your desk chair down the office luge track?
If you want to score the big prize, get to know the little guy.  
Play to win an NEC 61" plasma display: http://www.necitguy.com/?r 
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH RFC] Merge udev_scan_block() and udev_scan_class() in udevstart.c

[Tobias Klauser]

On 2005-06-06 at 00:01:18 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
> On Sun, Jun 05, 2005 at 03:00:16PM +0200, Tobias Klauser wrote:
> > udev_scan_block() and udev_scan_class() in udevstart.c have a lot of
> > redundant code and therefor I was thinking about a way to merge them.
> > My patch might not be the most elegant solution but it reduces the code
> > about 40 lines (see diffstat).
> 
> You will not get any class device with that patch! This prevents going
> down into the directories where the "dev" files are, it works only
> for block devices, cause they find a "dev" at the parent, but
> class-directories are empty here:
> 
>   if (has_devt(dirname))
>   	device_list_insert(dirname, type, &device_list);
>   else
>   	continue;
> 
> Please test more carefully before changing that bootup-critical parts of
> udev. The test directory contains a udevstart-test for that reason.

Sorry, that was stupid...

> There is a reason for these functions to be separate, but if you still think
> that this is worth to do, you may check the dirent type:
>   DTYPE(de) = DT_DIR
> instead of "dev" file lookups at every entry, which happens more often after
> merging these too functions.
> Anyway, I'm not convinced, that these functions are not better kept separate.

OK, if there's a reason for them to be separate I just didn't know it
while looking at the code.

Again, sorry for the noise.

Tobias


-------------------------------------------------------
This SF.Net email is sponsored by: NEC IT Guy Games.  How far can you shotput
a projector? How fast can you ride your desk chair down the office luge track?
If you want to score the big prize, get to know the little guy.  
Play to win an NEC 61" plasma display: http://www.necitguy.com/?r 
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel