Re: [RFC] [PATCH] Generate stable device names in device.map on Linux

["Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>]

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

>
> === modified file 'util/deviceiter.c'
> --- util/deviceiter.c	2010-06-11 20:31:16 +0000
> +++ util/deviceiter.c	2010-06-21 08:54:07 +0000
> @@ -28,6 +28,7 @@
>  #include <errno.h>
>  #include <fcntl.h>
>  #include <limits.h>
> +#include <dirent.h>
>  
>  #include <grub/util/misc.h>
>  #include <grub/util/deviceiter.h>
> @@ -345,18 +346,37 @@ get_xvd_disk_name (char *name, int unit)
>  }
>  #endif
>  
> +static struct seen_device
> +{
> +  struct seen_device *next;
> +  const char *name;
> +} *seen;
> +
>  /* Check if DEVICE can be read. If an error occurs, return zero,
>     otherwise return non-zero.  */
>  static int
>  check_device (const char *device)
>  {
>   
This patch subtly changes the semantics of this function. It's a static
one so the ramifications are relatively small. But you need at very
least to resync the comment and changing name when changing semantics is
recommended.
> @@ -441,6 +483,8 @@ grub_util_iterate_devices (int NESTED_FU
>  {
>    int i;
>  
> +  clear_seen_devices ();
> +
>    /* Floppies.  */
>    for (i = 0; i < floppy_disks; i++)
>      {
> @@ -453,10 +497,56 @@ grub_util_iterate_devices (int NESTED_FU
>        /* In floppies, write the map, whether check_device succeeds
>  	 or not, because the user just may not insert floppies.  */
>        if (hook (name, 1))
> -	return;
> +	goto out;
>      }
>  
>  #ifdef __linux__
> +  {
> +    DIR *dir = opendir ("/dev/disk/by-id");
> +
> +    if (dir)
> +      {
> +	struct dirent *entry;
> +	char **names;
> +	size_t names_len = 0, names_max = 1024, i;
> +
> +	names = xmalloc (names_max * sizeof *names);
> +
>   
Please use parentheses for sizeof
> +	qsort (names, names_len, sizeof *names, &compare_file_names);
> +
>   
Which part of code uses that array is sorted?
> +	closedir (dir);
> +
> +	/* Now add all the devices in sorted order.  */
> +	for (i = 0; i < names_len; ++i)
> +	  {
> +	    char *path = xasprintf ("/dev/disk/by-id/%s", names[i]);
> +	    if (check_device (path))
> +	      {
> +		if (hook (path, 0))
> +		  goto out;
> +	      }
> +	    free (path);
> +	    free (names[i]);
> +	  }
> +	free (names);
> +      }
> +  }
> +
>    if (have_devfs ())
>      {
>        i = 0;
>   


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]