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

[Vladimir 'φ-coder/phcoder' Serbinenko]

>
> === 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

signature.asc
Description: OpenPGP digital signature