Re: [PATCH] revert r1302

[Robert Millan]

Committed.

On Sun, Sep 07, 2008 at 06:59:08PM +0200, Robert Millan wrote:
> 
> Hi,
> 
> r1302 was a change I introduced to make device.map work on non-i386.  However,
> with time it appears that it wasn't really a good idea.  The powerpc version
> is fragile (see #465365 for an example of breakage), and nobody's really into
> maintaining the powerpc-ieee1275 port nowadays.
> 
> Then, at some point we started favouring UUIDs over hardcoded drive names in
> device.map.  As a result, even if your device.map names like hd0 are
> meaningless on ieee1275, this won't matter because grub.cfg uses UUIDs to
> access everything (and GRUB itself finds grub.cfg by asking the OFW).
> 
> So I propose reverting it (along with the changes that propagated to other
> files).  See attached patch.
> 
> -- 
> Robert Millan
> 
>   The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
>   how) you may access your data; but nobody's threatening your freedom: we
>   still allow you to remove your data and not access it at all."

> 2008-09-07  Robert Millan  <address@hidden>
> 
>       * conf/i386-pc.rmk (grub_mkdevicemap_SOURCES): Remove
>       `util/i386/get_disk_name.c'.
>       * conf/i386-efi.rmk: Likewise.
>       * conf/x86_64-efi.rmk: Likewise.
>       * conf/i386-coreboot.rmk: Likewise.
>       * conf/i386-ieee1275.rmk: Likewise.
>       * conf/powerpc-ieee1275.rmk (grub_mkdevicemap_SOURCES): Remove
>       `util/ieee1275/get_disk_name.c'.
>       * include/grub/util/misc.h (grub_util_get_disk_name): Remove.
>       * util/ieee1275/get_disk_name.c: Remove file.
>       * util/i386/get_disk_name.c: Remove file.
>       * util/grub-mkdevicemap.c (make_device_map): Back to hardcoding
>       "hd%d" for device.map entries, rather than using
>       grub_util_get_disk_name().
> 
> Index: conf/i386-pc.rmk
> ===================================================================
> --- conf/i386-pc.rmk  (revision 1857)
> +++ conf/i386-pc.rmk  (working copy)
> @@ -108,8 +108,7 @@ grub_setup_SOURCES = util/i386/pc/grub-s
>       grub_setup_init.c
>  
>  # For grub-mkdevicemap.
> -grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c util/misc.c               
> \
> -     util/i386/get_disk_name.c
> +grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c util/misc.c
>  
>  # For grub-emu.
>  util/grub-emu.c_DEPENDENCIES = grub_emu_init.h
> Index: conf/i386-efi.rmk
> ===================================================================
> --- conf/i386-efi.rmk (revision 1857)
> +++ conf/i386-efi.rmk (working copy)
> @@ -28,8 +28,7 @@ util/i386/efi/grub-mkimage.c_DEPENDENCIE
>  #    kern/fs.c kern/env.c fs/fshelp.c
>  
>  # For grub-mkdevicemap.
> -grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c util/misc.c               
> \
> -     util/i386/get_disk_name.c
> +grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c util/misc.c
>  
>  # For grub-emu.
>  util/grub-emu.c_DEPENDENCIES = grub_emu_init.h
> Index: conf/x86_64-efi.rmk
> ===================================================================
> --- conf/x86_64-efi.rmk       (revision 1857)
> +++ conf/x86_64-efi.rmk       (working copy)
> @@ -30,8 +30,7 @@ grub_mkimage_SOURCES = util/i386/efi/gru
>  #    kern/fs.c kern/env.c fs/fshelp.c
>  
>  # For grub-mkdevicemap.
> -grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c util/misc.c               
> \
> -     util/i386/get_disk_name.c
> +grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c util/misc.c
>  
>  # For grub-emu.
>  util/grub-emu.c_DEPENDENCIES = grub_emu_init.h
> Index: conf/i386-coreboot.rmk
> ===================================================================
> --- conf/i386-coreboot.rmk    (revision 1857)
> +++ conf/i386-coreboot.rmk    (working copy)
> @@ -51,8 +51,7 @@ sbin_UTILITIES += grub-emu
>  endif
>  
>  # For grub-mkdevicemap.
> -grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c util/misc.c               
> \
> -     util/i386/get_disk_name.c
> +grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c util/misc.c
>  
>  # For grub-emu.
>  util/grub-emu.c_DEPENDENCIES = grub_emu_init.h
> Index: conf/powerpc-ieee1275.rmk
> ===================================================================
> --- conf/powerpc-ieee1275.rmk (revision 1856)
> +++ conf/powerpc-ieee1275.rmk (working copy)
> @@ -34,8 +34,7 @@ sbin_UTILITIES += grub-emu
>  endif
>   
>  # For grub-mkdevicemap.
> -grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c util/misc.c               
> \
> -     util/ieee1275/get_disk_name.c
> +grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c util/misc.c
>  
>  # For grub-emu
>  util/grub-emu.c_DEPENDENCIES = grub_emu_init.h
> Index: conf/i386-ieee1275.rmk
> ===================================================================
> --- conf/i386-ieee1275.rmk    (revision 1857)
> +++ conf/i386-ieee1275.rmk    (working copy)
> @@ -48,8 +48,7 @@ sbin_UTILITIES += grub-emu
>  endif
>  
>  # For grub-mkdevicemap.
> -grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c util/misc.c               
> \
> -     util/i386/get_disk_name.c
> +grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c util/misc.c
>  
>  # For grub-emu.
>  util/grub-emu.c_DEPENDENCIES = grub_emu_init.h
> Index: include/grub/util/misc.h
> ===================================================================
> --- include/grub/util/misc.h  (revision 1856)
> +++ include/grub/util/misc.h  (working copy)
> @@ -56,7 +56,6 @@ void grub_util_load_image (const char *p
>  void grub_util_write_image (const char *img, size_t size, FILE *out);
>  void grub_util_write_image_at (const void *img, size_t size, off_t offset,
>                              FILE *out);
> -char *grub_util_get_disk_name (int disk, char *name);
>  
>  #ifndef  HAVE_ASPRINTF
>  
> Index: util/ieee1275/get_disk_name.c
> ===================================================================
> --- util/ieee1275/get_disk_name.c     (revision 1856)
> +++ util/ieee1275/get_disk_name.c     (working copy)
> @@ -1,61 +0,0 @@
> -/* get_disk_name.c */
> -/*
> - *  GRUB  --  GRand Unified Bootloader
> - *  Copyright (C) 2007 Free Software Foundation, Inc.
> - *
> - *  GRUB is free software: you can redistribute it and/or modify
> - *  it under the terms of the GNU General Public License as published by
> - *  the Free Software Foundation, either version 3 of the License, or
> - *  (at your option) any later version.
> - *
> - *  GRUB is distributed in the hope that it will be useful,
> - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - *  GNU General Public License for more details.
> - *
> - *  You should have received a copy of the GNU General Public License
> - *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> - */
> -
> -#include <config.h>
> -#include <stdio.h>
> -#include <grub/util/misc.h>
> -
> -char *
> -grub_util_get_disk_name (int disk __attribute__ ((unused)), char *name)
> -{
> -  int p[2];
> -
> -  char *line = NULL;
> -  size_t zero = 0;
> -  int len;
> -
> -  pipe (p);
> -
> -  switch (fork ())
> -    {
> -    case -1:
> -      perror ("fork");
> -      exit (1);
> -    case 0:
> -      close (1);
> -      dup (p[1]);
> -      close (p[0]);
> -      close (p[1]);
> -      execlp ("ofpathname", "ofpathname", "-a", name, NULL);
> -      perror ("execlp");
> -    default:
> -      close (0);
> -      dup (p[0]);
> -      close (p[0]);
> -      close (p[1]);
> -    }
> -
> -  len = getline (&line, &zero, stdin);
> -  if (len < 2)
> -    grub_util_error ("ofpathname didn't print a meaningful alias name");
> -
> -  line[len - 1] = '\0';
> -  
> -  return line;
> -}
> Index: util/i386/get_disk_name.c
> ===================================================================
> --- util/i386/get_disk_name.c (revision 1856)
> +++ util/i386/get_disk_name.c (working copy)
> @@ -1,31 +0,0 @@
> -/* get_disk_name.c */
> -/*
> - *  GRUB  --  GRand Unified Bootloader
> - *  Copyright (C) 2007 Free Software Foundation, Inc.
> - *
> - *  GRUB is free software: you can redistribute it and/or modify
> - *  it under the terms of the GNU General Public License as published by
> - *  the Free Software Foundation, either version 3 of the License, or
> - *  (at your option) any later version.
> - *
> - *  GRUB is distributed in the hope that it will be useful,
> - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - *  GNU General Public License for more details.
> - *
> - *  You should have received a copy of the GNU General Public License
> - *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> - */
> -
> -#include <grub/util/misc.h>
> -
> -char *
> -grub_util_get_disk_name (int disk, char *name  __attribute__ ((unused)))
> -{
> -  char *p;
> -
> -  p = xmalloc (16);
> -  sprintf (p, "hd%d", disk);
> -  
> -  return p;
> -}
> Index: util/grub-mkdevicemap.c
> ===================================================================
> --- util/grub-mkdevicemap.c   (revision 1856)
> +++ util/grub-mkdevicemap.c   (working copy)
> @@ -450,11 +450,8 @@ make_device_map (const char *device_map,
>         
>         if (realpath (discn, name))
>           {
> -           char *p;
>             strcat (name, "/disc");
> -           p = grub_util_get_disk_name (num_hd, name);
> -           fprintf (fp, "(%s)\t%s\n", p, name);
> -           free (p);
> +           fprintf (fp, "(hd%d)\t%s\n", num_hd, name);
>           }
>         
>         num_hd++;
> @@ -472,10 +469,7 @@ make_device_map (const char *device_map,
>        get_ide_disk_name (name, i);
>        if (check_device (name))
>       {
> -       char *p;
> -       p = grub_util_get_disk_name (num_hd, name);
> -       fprintf (fp, "(%s)\t%s\n", p, name);
> -       free (p);
> +       fprintf (fp, "(hd%d)\t%s\n", num_hd, name);
>         num_hd++;
>       }
>      }
> @@ -505,10 +499,7 @@ make_device_map (const char *device_map,
>        get_ataraid_disk_name (name, i);
>        if (check_device (name))
>       {
> -       char *p;
> -       p = grub_util_get_disk_name (num_hd, name);
> -       fprintf (fp, "(%s)\t%s\n", p, name);
> -       free (p);
> +       fprintf (fp, "(hd%d)\t%s\n", num_hd, name);
>            num_hd++;
>          }
>      }
> @@ -538,10 +529,7 @@ make_device_map (const char *device_map,
>        get_scsi_disk_name (name, i);
>        if (check_device (name))
>       {
> -       char *p;
> -       p = grub_util_get_disk_name (num_hd, name);
> -       fprintf (fp, "(%s)\t%s\n", p, name);
> -       free (p);
> +       fprintf (fp, "(hd%d)\t%s\n", num_hd, name);
>         num_hd++;
>       }
>      }
> @@ -564,10 +552,7 @@ make_device_map (const char *device_map,
>           get_dac960_disk_name (name, controller, drive);
>           if (check_device (name))
>             {
> -             char *p;
> -             p = grub_util_get_disk_name (num_hd, name);
> -             fprintf (fp, "(%s)\t%s\n", p, name);
> -             free (p);
> +             fprintf (fp, "(hd%d)\t%s\n", num_hd, name);
>               num_hd++;
>             }
>         }
> @@ -633,10 +618,7 @@ make_device_map (const char *device_map,
>       get_i2o_disk_name (name, unit);
>       if (check_device (name))
>         {
> -         char *p;
> -         p = grub_util_get_disk_name (num_hd, name);
> -         fprintf (fp, "(%s)\t%s\n", p, name);
> -         free (p);
> +         fprintf (fp, "(hd%d)\t%s\n", num_hd, name);
>           num_hd++;
>         }
>        }

> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel


-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."