* grub-install --root-directory=/mnt /dev/sda1 fails
@ 2009-05-06 14:31 Felix Zielcke
2009-05-06 15:12 ` Vladimir 'phcoder' Serbinenko
0 siblings, 1 reply; 14+ messages in thread
From: Felix Zielcke @ 2009-05-06 14:31 UTC (permalink / raw)
To: The development of GRUB 2
Hello list,
grub-install with some --root-directory option and a partition given as
setup device and not a whole disk doestn't work, i.e. blocklists are
used.
grub-setup fails that it can't find the core.img.
The problem are these 2 lines in setup():
core_path_dev = grub_util_get_path (dir, core_file);
...
file = grub_file_open (core_path_dev);
core_path_dev contains the full path given with the `-d' or
`--directory' option for example /mnt/boot/grub.
This can't work because /mnt is a different filesystem and so GRUB only
sees /boot/grub.
Removing the /mnt part from it works fine:
(gdb) set core_path_dev="/boot/grub/core.img"
(gdb) print *file->device->disk
$13 = {name = 0x640b90 "hd1,1", dev = 0x62f520, total_sectors = 16777216, has_partitions = 1, id = 1, partition = 0x640c10, read_hook = 0, data = 0x0}
The only solution that comes to my mind for this problem is, to
introduce a `--root-directory' option and making the `--directory'
option relative to it.
Does anyone have a better idea?
--
Felix Zielcke
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: grub-install --root-directory=/mnt /dev/sda1 fails 2009-05-06 14:31 grub-install --root-directory=/mnt /dev/sda1 fails Felix Zielcke @ 2009-05-06 15:12 ` Vladimir 'phcoder' Serbinenko 2009-06-01 19:39 ` Felix Zielcke 0 siblings, 1 reply; 14+ messages in thread From: Vladimir 'phcoder' Serbinenko @ 2009-05-06 15:12 UTC (permalink / raw) To: The development of GRUB 2 [-- Attachment #1: Type: text/plain, Size: 1459 bytes --] On Wed, May 6, 2009 at 4:31 PM, Felix Zielcke <fzielcke@z-51.de> wrote: > Hello list, > > grub-install with some --root-directory option and a partition given as > setup device and not a whole disk doestn't work, i.e. blocklists are > used. > grub-setup fails that it can't find the core.img. > > The problem are these 2 lines in setup(): > core_path_dev = grub_util_get_path (dir, core_file); > ... > file = grub_file_open (core_path_dev); > > core_path_dev contains the full path given with the `-d' or > `--directory' option for example /mnt/boot/grub. > > This can't work because /mnt is a different filesystem and so GRUB only > sees /boot/grub. > Removing the /mnt part from it works fine: > (gdb) set core_path_dev="/boot/grub/core.img" > (gdb) print *file->device->disk > $13 = {name = 0x640b90 "hd1,1", dev = 0x62f520, total_sectors = 16777216, > has_partitions = 1, id = 1, partition = 0x640c10, read_hook = 0, data = 0x0} > > The only solution that comes to my mind for this problem is, to > introduce a `--root-directory' option and making the `--directory' > option relative to it. > > Does anyone have a better idea? Don't we already have a function which transforms host directory into grub directory? AFAIR we have. > > -- > Felix Zielcke > > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > -- Regards Vladimir 'phcoder' Serbinenko [-- Attachment #2: Type: text/html, Size: 2197 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: grub-install --root-directory=/mnt /dev/sda1 fails 2009-05-06 15:12 ` Vladimir 'phcoder' Serbinenko @ 2009-06-01 19:39 ` Felix Zielcke 2009-06-08 21:20 ` [PATCH] " Felix Zielcke 2009-07-01 14:33 ` Felix Zielcke 0 siblings, 2 replies; 14+ messages in thread From: Felix Zielcke @ 2009-06-01 19:39 UTC (permalink / raw) To: The development of GRUB 2 [-- Attachment #1: Type: text/plain, Size: 670 bytes --] Am Mittwoch, den 06.05.2009, 17:12 +0200 schrieb Vladimir 'phcoder' Serbinenko: > Don't we already have a function which transforms host directory into > grub > directory? AFAIR we have. There's just the shell function in grub-mkconfig_lib.in Here's now a patch wich implements it in util/hostdisk.c and gets used for core_path_dev in setup (). But it doestn't work with symlinks. readlink () can only be used if the file pointed to is a symlink, not if a symlink is somewhere in between. coreutils where the readlink binary is from is GPL 3+ but the function for it uses hash tables and it seems like it would be too much code to copy just for this. -- Felix Zielcke [-- Attachment #2: make_sys_path_relative.patch --] [-- Type: text/x-patch, Size: 2244 bytes --] 2009-06-01 Felix Zielcke <fzielcke@z-51.de> * include/grub/util/hostdisk.c (grub_make_system_path_relative_to_its_root): New function prototype. * util/hostdisk.c (grub_make_system_path_relative_to_its_root): New function. * util/i386/pc/grub-setup.c (setup): Use grub_make_system_path_relative_to_its_root to make core_path_dev relative to the partition. diff --git a/include/grub/util/hostdisk.h b/include/grub/util/hostdisk.h index 21efb0d..0fb0219 100644 --- a/include/grub/util/hostdisk.h +++ b/include/grub/util/hostdisk.h @@ -23,5 +23,7 @@ void grub_util_biosdisk_init (const char *dev_map); void grub_util_biosdisk_fini (void); char *grub_util_biosdisk_get_grub_dev (const char *os_dev); +char *grub_make_system_path_relative_to_its_root (char *path); + #endif /* ! GRUB_BIOSDISK_MACHINE_UTIL_HEADER */ diff --git a/util/hostdisk.c b/util/hostdisk.c index eaccb73..2b80b48 100644 --- a/util/hostdisk.c +++ b/util/hostdisk.c @@ -1072,3 +1072,39 @@ grub_util_biosdisk_get_grub_dev (const char *os_dev) return make_device_name (drive, -1, -1); #endif } + +char *grub_make_system_path_relative_to_its_root (char *path) +{ + + struct stat st; + char buf[500], buf2[500]; + dev_t num; + char *p; + + if (stat (path, &st) < 0) + return path; + + num = st.st_dev; + memset (buf, 0 , sizeof (buf)); + strncpy (buf, path, 500); + strcpy (buf2, buf); + while (1) + { + p = strrchr (buf, '/'); + if (p != buf) + *p = 0; + else *++p = 0; + + if (stat (buf, &st) < 0) + return path; + + if (st.st_dev != num) + break; + strcpy(buf2,buf); + if (p - 1 == buf) + return path; + } + for (p = buf2; *p != 0; p++) + path++; + return path; +} diff --git a/util/i386/pc/grub-setup.c b/util/i386/pc/grub-setup.c index 997811b..57f49d4 100644 --- a/util/i386/pc/grub-setup.c +++ b/util/i386/pc/grub-setup.c @@ -405,6 +405,7 @@ unable_to_embed: /* Make sure that GRUB reads the identical image as the OS. */ tmp_img = xmalloc (core_size); core_path_dev = grub_util_get_path (dir, core_file); + core_path_dev = grub_make_system_path_relative_to_its_root (core_path_dev); /* It is a Good Thing to sync two times. */ sync (); ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH] Re: grub-install --root-directory=/mnt /dev/sda1 fails 2009-06-01 19:39 ` Felix Zielcke @ 2009-06-08 21:20 ` Felix Zielcke 2009-06-09 21:51 ` Vladimir 'phcoder' Serbinenko 2009-07-01 14:33 ` Felix Zielcke 1 sibling, 1 reply; 14+ messages in thread From: Felix Zielcke @ 2009-06-08 21:20 UTC (permalink / raw) To: The development of GRUB 2 [-- Attachment #1: Type: text/plain, Size: 826 bytes --] Am Montag, den 01.06.2009, 21:39 +0200 schrieb Felix Zielcke: > Am Mittwoch, den 06.05.2009, 17:12 +0200 schrieb Vladimir 'phcoder' > Serbinenko: > > Don't we already have a function which transforms host directory into > > grub > > directory? AFAIR we have. > > There's just the shell function in grub-mkconfig_lib.in > Here's now a patch wich implements it in util/hostdisk.c and gets used > for core_path_dev in setup (). > But it doestn't work with symlinks. > readlink () can only be used if the file pointed to is a symlink, not if > a symlink is somewhere in between. > coreutils where the readlink binary is from is GPL 3+ but the function > for it uses hash tables and it seems like it would be too much code to > copy just for this. Here's a new patch which prints out an error if stat () fails. -- Felix Zielcke [-- Attachment #2: make_sys_path_relative.patch.2 --] [-- Type: text/plain, Size: 1890 bytes --] 2009-06-08 Felix Zielcke <fzielcke@z-51.de> * include/grub/util/hostdisk.c (grub_make_system_path_relative_to_its_root): New function prototype. * util/hostdisk.c (grub_make_system_path_relative_to_its_root): New function. * util/i386/pc/grub-setup.c (setup): Use grub_make_system_path_relative_to_its_root to make core_path_dev relative to the partition. diff --git a/util/hostdisk.c b/util/hostdisk.c index a7262dd..0a786ba 100644 --- a/util/hostdisk.c +++ b/util/hostdisk.c @@ -1076,3 +1076,39 @@ grub_util_biosdisk_get_grub_dev (const char *os_dev) return make_device_name (drive, -1, -1); #endif } + +char *grub_make_system_path_relative_to_its_root (char *path) +{ + + struct stat st; + char buf[500], buf2[500]; + dev_t num; + char *p; + + if (stat (path, &st) < 0) + return NULL; + + num = st.st_dev; + memset (buf, 0 , sizeof (buf)); + strncpy (buf, path, 500); + strcpy (buf2, buf); + while (1) + { + p = strrchr (buf, '/'); + if (p != buf) + *p = 0; + else *++p = 0; + + if (stat (buf, &st) < 0) + return NULL; + + if (st.st_dev != num) + break; + strcpy(buf2,buf); + if (p - 1 == buf) + return path; + } + for (p = buf2; *p != 0; p++) + path++; + return path; +} diff --git a/util/i386/pc/grub-setup.c b/util/i386/pc/grub-setup.c index 997811b..9446fd5 100644 --- a/util/i386/pc/grub-setup.c +++ b/util/i386/pc/grub-setup.c @@ -405,6 +405,9 @@ unable_to_embed: /* Make sure that GRUB reads the identical image as the OS. */ tmp_img = xmalloc (core_size); core_path_dev = grub_util_get_path (dir, core_file); + core_path_dev = grub_make_system_path_relative_to_its_root (core_path_dev); + if (core_path_dev == NULL) + grub_util_error ("failed to make path of core.img relative to its root"); /* It is a Good Thing to sync two times. */ sync (); ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] Re: grub-install --root-directory=/mnt /dev/sda1 fails 2009-06-08 21:20 ` [PATCH] " Felix Zielcke @ 2009-06-09 21:51 ` Vladimir 'phcoder' Serbinenko 2009-06-10 23:00 ` Felix Zielcke 0 siblings, 1 reply; 14+ messages in thread From: Vladimir 'phcoder' Serbinenko @ 2009-06-09 21:51 UTC (permalink / raw) To: The development of GRUB 2 2009-06-08 Felix Zielcke <fzielcke@z-51.de> * include/grub/util/hostdisk.c (grub_make_system_path_relative_to_its_root): New function prototype. * util/hostdisk.c (grub_make_system_path_relative_to_its_root): New function. * util/i386/pc/grub-setup.c (setup): Use grub_make_system_path_relative_to_its_root to make core_path_dev relative to the partition. diff --git a/util/hostdisk.c b/util/hostdisk.c index a7262dd..0a786ba 100644 --- a/util/hostdisk.c +++ b/util/hostdisk.c @@ -1076,3 +1076,39 @@ grub_util_biosdisk_get_grub_dev (const char *os_dev) return make_device_name (drive, -1, -1); #endif } + +char *grub_make_system_path_relative_to_its_root (char *path) +{ + + struct stat st; + char buf[500], buf2[500]; Use malloc instead of static allocation + p = strrchr (buf, '/'); + if (p != buf) + *p = 0; + else *++p = 0; You assume path starts with /. You have to check this. Otherwise you may get sigsegv + strcpy(buf2,buf); Just save (p - buf) instead of copying buf to buf2 On Mon, Jun 8, 2009 at 11:20 PM, Felix Zielcke<fzielcke@z-51.de> wrote: > Am Montag, den 01.06.2009, 21:39 +0200 schrieb Felix Zielcke: >> Am Mittwoch, den 06.05.2009, 17:12 +0200 schrieb Vladimir 'phcoder' >> Serbinenko: >> > Don't we already have a function which transforms host directory into >> > grub >> > directory? AFAIR we have. >> >> There's just the shell function in grub-mkconfig_lib.in >> Here's now a patch wich implements it in util/hostdisk.c and gets used >> for core_path_dev in setup (). >> But it doestn't work with symlinks. >> readlink () can only be used if the file pointed to is a symlink, not if >> a symlink is somewhere in between. >> coreutils where the readlink binary is from is GPL 3+ but the function >> for it uses hash tables and it seems like it would be too much code to >> copy just for this. > > Here's a new patch which prints out an error if stat () fails. > -- > Felix Zielcke > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > > -- Regards Vladimir 'phcoder' Serbinenko ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] Re: grub-install --root-directory=/mnt /dev/sda1 fails 2009-06-09 21:51 ` Vladimir 'phcoder' Serbinenko @ 2009-06-10 23:00 ` Felix Zielcke 2009-06-11 23:21 ` Felix Zielcke 0 siblings, 1 reply; 14+ messages in thread From: Felix Zielcke @ 2009-06-10 23:00 UTC (permalink / raw) To: The development of GRUB 2 [-- Attachment #1: Type: text/plain, Size: 1157 bytes --] Am Dienstag, den 09.06.2009, 23:51 +0200 schrieb Vladimir 'phcoder' Serbinenko: > + > +char *grub_make_system_path_relative_to_its_root (char *path) > +{ > + > + struct stat st; > + char buf[500], buf2[500]; > Use malloc instead of static allocation Changed. > + p = strrchr (buf, '/'); > + if (p != buf) > + *p = 0; > + else *++p = 0; > You assume path starts with /. You have to check this. Otherwise you > may get sigsegv Changed. > + strcpy(buf2,buf); > Just save (p - buf) instead of copying buf to buf2 I did this now if realpath () isn't avaible. Now realpath is used in case it's avaible. POSIX specifies it and readlink is actually using it if it's avaible. I didn't read the source correctly. The problem is just Cygwin. It has it but it returns the cygwin path. So C:\\Windows would get /cygdrive/c/windows, which is easy to handle. But realpath ("/boot/grub") would return /boot/grub which isn't true from Windows/GRUB point of view. Maybe Christian and Bean can say something about it. MingW doestn't have it and I don't know what Windows would have, so the MingW users would still use my own way. -- Felix Zielcke [-- Attachment #2: make_sys_path_relative.patch.3 --] [-- Type: text/plain, Size: 4504 bytes --] 2009-06-11 Felix Zielcke <fzielcke@z-51.de> * configure.ac (AC_CHECK_FUNCS): Add realpath. * include/grub/util/hostdisk.c (grub_make_system_path_relative_to_its_root): New function prototype. * util/hostdisk.c: Include <stdint.h>. (grub_make_system_path_relative_to_its_root): New function. * util/i386/pc/grub-setup.c (setup): Use grub_make_system_path_relative_to_its_root to make core_path_dev relative to the partition. diff --git a/configure.ac b/configure.ac index e448c2f..9e923a6 100644 --- a/configure.ac +++ b/configure.ac @@ -205,7 +205,7 @@ if test "$target_cpu"-"$platform" = i386-pc; then fi # Check for functions. -AC_CHECK_FUNCS(posix_memalign memalign asprintf) +AC_CHECK_FUNCS(posix_memalign memalign asprintf realpath) # # Check for target programs. diff --git a/include/grub/util/hostdisk.h b/include/grub/util/hostdisk.h index 21efb0d..a1ecf59 100644 --- a/include/grub/util/hostdisk.h +++ b/include/grub/util/hostdisk.h @@ -23,5 +23,6 @@ void grub_util_biosdisk_init (const char *dev_map); void grub_util_biosdisk_fini (void); char *grub_util_biosdisk_get_grub_dev (const char *os_dev); +char *grub_make_system_path_relative_to_its_root (char *path) #endif /* ! GRUB_BIOSDISK_MACHINE_UTIL_HEADER */ diff --git a/util/hostdisk.c b/util/hostdisk.c index a7262dd..bce1b95 100644 --- a/util/hostdisk.c +++ b/util/hostdisk.c @@ -27,6 +27,7 @@ #include <grub/misc.h> #include <stdio.h> +#include <stdint.h> #include <stdlib.h> #include <string.h> #include <ctype.h> @@ -1076,3 +1077,94 @@ grub_util_biosdisk_get_grub_dev (const char *os_dev) return make_device_name (drive, -1, -1); #endif } + +char * +grub_make_system_path_relative_to_its_root (char *path) +{ + struct stat st; + char *buf, *buf2; + uintptr_t offset = 0; + dev_t num; + long path_max; +#ifdef HAVE_REALPATH + char *p; +#endif + + +#ifdef HAVE_REALPATH +# ifdef PATH_MAX + path_max = PATH_MAX +# else + path_max = pathconf (path, _PC_PATH_MAX); + if (path_max <= 0) + /* 1024 is taken from glibc-2.10. */ + path_max = 1024; +# endif + p = xmalloc (path_max); + p = realpath (path, p); + if (p == NULL) + grub_util_error ("failed to get realpath of %s", path); + +#ifdef __CYGWIN__ + if (strncmp (p,"/cygdrive/") == 0) + { + p += sizeof "/cygdrive/c" - 1; + } + else + grub_util_error "path not under /cygdrive/ aborting."; +#endif + buf = xmalloc (path_max); + buf2 = xmalloc (path_max); + strcpy (buf, p); + free (p); +#else /* ! HAVE_REALPATH */ +# warning "The function `grub_make_system_path_relative_to_its_root' might not work on your OS correctly." + path_max = 1024; + buf = xmalloc (path_max); + buf2 = xmalloc (path_max); + memset (buf, 0, sizeof (buf)); + if (*path != '/') + { + size_t len = strlen (path); + + if (getcwd (buf, 1024 - len) == NULL) + grub_util_error ("can not get current working directory"); + + strcat (buf, "/"); + strcat (buf, path); + } + else + { + if (strlen (path) > 1024) + grub_util_error ("path too long"); + strncpy (buf, path, 1024); + } +#endif /* ! HAVE_REALPATH */ + + strcpy (buf2, buf); + if (stat (buf, &st) < 0) + grub_util_error ("can not stat %s", p); + + num = st.st_dev; + while (1) + { + p = strrchr (buf, '/'); + if (p == NULL) + grub_util_error ("FIXME no / in p"); + if (p != buf) + *p = 0; + else + *++p = 0; + + if (stat (buf, &st) < 0) + grub_util_error ("can not stat %s", buf); + + if (st.st_dev != num) + break; + + offset = p - buf; + if (offset == 1) + return buf2; + } + return buf2 + offset; +} diff --git a/util/i386/pc/grub-setup.c b/util/i386/pc/grub-setup.c index 997811b..7fb0273 100644 --- a/util/i386/pc/grub-setup.c +++ b/util/i386/pc/grub-setup.c @@ -89,7 +89,7 @@ setup (const char *dir, const char *root, const char *dest, int must_embed, int force) { char *boot_path, *core_path, *core_path_dev; - char *boot_img, *core_img; + char *boot_img, *core_img, *p; size_t boot_size, core_size; grub_uint16_t core_sectors; grub_device_t root_dev, dest_dev; @@ -404,7 +404,9 @@ unable_to_embed: /* Make sure that GRUB reads the identical image as the OS. */ tmp_img = xmalloc (core_size); - core_path_dev = grub_util_get_path (dir, core_file); + p = grub_util_get_path (dir, core_file); + core_path_dev = grub_make_system_path_relative_to_its_root (p); + free (p); /* It is a Good Thing to sync two times. */ sync (); ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] Re: grub-install --root-directory=/mnt /dev/sda1 fails 2009-06-10 23:00 ` Felix Zielcke @ 2009-06-11 23:21 ` Felix Zielcke 2009-06-11 23:25 ` Felix Zielcke 0 siblings, 1 reply; 14+ messages in thread From: Felix Zielcke @ 2009-06-11 23:21 UTC (permalink / raw) To: The development of GRUB 2 [-- Attachment #1: Type: text/plain, Size: 1495 bytes --] Am Donnerstag, den 11.06.2009, 01:00 +0200 schrieb Felix Zielcke: > Am Dienstag, den 09.06.2009, 23:51 +0200 schrieb Vladimir 'phcoder' > Serbinenko: > > > + > > +char *grub_make_system_path_relative_to_its_root (char *path) > > +{ > > + > > + struct stat st; > > + char buf[500], buf2[500]; > > Use malloc instead of static allocation > > Changed. > > > + p = strrchr (buf, '/'); > > + if (p != buf) > > + *p = 0; > > + else *++p = 0; > > You assume path starts with /. You have to check this. Otherwise you > > may get sigsegv > > Changed. > > + strcpy(buf2,buf); > > Just save (p - buf) instead of copying buf to buf2 > > I did this now if realpath () isn't avaible. > Now realpath is used in case it's avaible. POSIX specifies it and > readlink is actually using it if it's avaible. I didn't read the source > correctly. > The problem is just Cygwin. It has it but it returns the cygwin path. > So C:\\Windows would get /cygdrive/c/windows, which is easy to handle. > But realpath ("/boot/grub") would return /boot/grub which isn't true > from Windows/GRUB point of view. > Maybe Christian and Bean can say something about it. > MingW doestn't have it and I don't know what Windows would have, so the > MingW users would still use my own way. Here's now a new one which aborts if realpath is avaible but doestn't support (path, NULL) and the fallback function is changed to dynamically allocate the memory. And now all memory is properly free'd -- Felix Zielcke [-- Attachment #2: make_sys_path_relative.patch.5 --] [-- Type: text/plain, Size: 4560 bytes --] 2009-06-11 Felix Zielcke <fzielcke@z-51.de> * configure.ac (AC_CHECK_FUNCS): Add realpath. * include/grub/util/hostdisk.c (grub_make_system_path_relative_to_its_root): New function prototype. * util/hostdisk.c: Include <stdint.h>. (grub_make_system_path_relative_to_its_root): New function. * util/i386/pc/grub-setup.c (setup): Use grub_make_system_path_relative_to_its_root to make core_path_dev relative to the partition. diff --git a/configure.ac b/configure.ac index b0f7bb7..9d594db 100644 --- a/configure.ac +++ b/configure.ac @@ -196,7 +196,7 @@ if test "$target_cpu"-"$platform" = i386-pc; then fi # Check for functions. -AC_CHECK_FUNCS(posix_memalign memalign asprintf) +AC_CHECK_FUNCS(posix_memalign memalign asprintf realpath) # # Check for target programs. diff --git a/include/grub/util/hostdisk.h b/include/grub/util/hostdisk.h index 21efb0d..85df1c6 100644 --- a/include/grub/util/hostdisk.h +++ b/include/grub/util/hostdisk.h @@ -23,5 +23,6 @@ void grub_util_biosdisk_init (const char *dev_map); void grub_util_biosdisk_fini (void); char *grub_util_biosdisk_get_grub_dev (const char *os_dev); +char *grub_make_system_path_relative_to_its_root (char *path,void **free_ptr); #endif /* ! GRUB_BIOSDISK_MACHINE_UTIL_HEADER */ diff --git a/util/hostdisk.c b/util/hostdisk.c index 1844a7e..3224e07 100644 --- a/util/hostdisk.c +++ b/util/hostdisk.c @@ -27,6 +27,7 @@ #include <grub/misc.h> #include <stdio.h> +#include <stdint.h> #include <stdlib.h> #include <string.h> #include <ctype.h> @@ -1076,3 +1077,95 @@ grub_util_biosdisk_get_grub_dev (const char *os_dev) return make_device_name (drive, -1, -1); #endif } + +char * +grub_make_system_path_relative_to_its_root (char *path, void **free_ptr) +{ + struct stat st; + char *buf, *buf2; + uintptr_t offset = 0; + dev_t num; + long path_max; + size_t len, len2; + char *p; + + +#ifdef HAVE_REALPATH + p = realpath (path, NULL); + + if (p == NULL) && (ernno != ) + grub_util_error ("failed to get realpath of %s", path); + else + grub_util_error ("realpath not supporting (path, NULL)"); + len = strlen (p) + 1; + buf = xmalloc (len); + buf2 = xmalloc (len); +# ifdef __CYGWIN__ + if (strncmp (p, 10, "/cygdrive/") == 0) + strcpy (buf, p + sizeof ("/cygdrive/c") - 1); + else + grub_util_error "path not under /cygdrive/. Aborting."; +# else + strcpy (buf, p); +#endif + free (p); +#else /* ! HAVE_REALPATH */ +# warning "The function `grub_make_system_path_relative_to_its_root' might not work on your OS correctly." + if (*path != '/') + { + len2 = 4096; + do + { + p = getcwd (buf, len) + if (p == NULL) + { + if (errno != ERANGE) + grub_util_error ("can not get current working directory"); + else + len2 *= 2; + } + } while (p == NULL) + + buf = xmalloc (strlen (path) + len2 + 1); + strcat (buf, "/"); + strcat (buf, path); + } + else + { + buf = xmalloc (strlen (path) + 1); + strcpy (buf, path) + } +#endif /* ! HAVE_REALPATH */ + buf2 = xmalloc (strlen (buf) + 1); + strcpy (buf2, buf); + *free_ptr = buf2; + if (stat (buf, &st) < 0) + grub_util_error ("can not stat %s", p); + + num = st.st_dev; + while (1) + { + p = strrchr (buf, '/'); + if (p == NULL) + grub_util_error ("FIXME no / in buf"); + if (p != buf) + *p = 0; + else + *++p = 0; + + if (stat (buf, &st) < 0) + grub_util_error ("can not stat %s", buf); + + if (st.st_dev != num) + break; + + offset = p - buf; + if (offset == 1) + { + free (buf); + return buf2; + } + } + free (buf); + return buf2 + offset; +} diff --git a/util/i386/pc/grub-setup.c b/util/i386/pc/grub-setup.c index bdf234c..c9243bd 100644 --- a/util/i386/pc/grub-setup.c +++ b/util/i386/pc/grub-setup.c @@ -89,7 +89,7 @@ setup (const char *dir, const char *root, const char *dest, int must_embed, int force) { char *boot_path, *core_path, *core_path_dev; - char *boot_img, *core_img; + char *boot_img, *core_img, *p, *free_ptr; size_t boot_size, core_size; grub_uint16_t core_sectors; grub_device_t root_dev, dest_dev; @@ -404,7 +404,10 @@ unable_to_embed: /* Make sure that GRUB reads the identical image as the OS. */ tmp_img = xmalloc (core_size); - core_path_dev = grub_util_get_path (dir, core_file); + p = grub_util_get_path (dir, core_file); + core_path_dev = grub_make_system_path_relative_to_its_root (p,&free_ptr); + free (p); + free (free_ptr); /* It is a Good Thing to sync two times. */ sync (); ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] Re: grub-install --root-directory=/mnt /dev/sda1 fails 2009-06-11 23:21 ` Felix Zielcke @ 2009-06-11 23:25 ` Felix Zielcke 2009-06-12 10:28 ` Felix Zielcke 0 siblings, 1 reply; 14+ messages in thread From: Felix Zielcke @ 2009-06-11 23:25 UTC (permalink / raw) To: The development of GRUB 2 Am Freitag, den 12.06.2009, 01:21 +0200 schrieb Felix Zielcke: > Am Donnerstag, den 11.06.2009, 01:00 +0200 schrieb Felix Zielcke: > > Am Dienstag, den 09.06.2009, 23:51 +0200 schrieb Vladimir 'phcoder' > > Serbinenko: > > > > > + > > > +char *grub_make_system_path_relative_to_its_root (char *path) > > > +{ > > > + > > > + struct stat st; > > > + char buf[500], buf2[500]; > > > Use malloc instead of static allocation > > > > Changed. > > > > > + p = strrchr (buf, '/'); > > > + if (p != buf) > > > + *p = 0; > > > + else *++p = 0; > > > You assume path starts with /. You have to check this. Otherwise you > > > may get sigsegv > > > > Changed. > > > + strcpy(buf2,buf); > > > Just save (p - buf) instead of copying buf to buf2 > > > > I did this now if realpath () isn't avaible. > > Now realpath is used in case it's avaible. POSIX specifies it and > > readlink is actually using it if it's avaible. I didn't read the source > > correctly. > > The problem is just Cygwin. It has it but it returns the cygwin path. > > So C:\\Windows would get /cygdrive/c/windows, which is easy to handle. > > But realpath ("/boot/grub") would return /boot/grub which isn't true > > from Windows/GRUB point of view. > > Maybe Christian and Bean can say something about it. > > MingW doestn't have it and I don't know what Windows would have, so the > > MingW users would still use my own way. > > Here's now a new one which aborts if realpath is avaible but doestn't > support (path, NULL) > and the fallback function is changed to dynamically allocate the memory. > And now all memory is properly free'd args just noticed I forgot the *free_ptr = buf2; I'm too lazy now to send an extra patch just for this. -- Felix Zielcke ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Re: grub-install --root-directory=/mnt /dev/sda1 fails 2009-06-11 23:25 ` Felix Zielcke @ 2009-06-12 10:28 ` Felix Zielcke 2009-06-12 16:21 ` Pavel Roskin 0 siblings, 1 reply; 14+ messages in thread From: Felix Zielcke @ 2009-06-12 10:28 UTC (permalink / raw) To: The development of GRUB 2 [-- Attachment #1: Type: text/plain, Size: 1952 bytes --] Am Freitag, den 12.06.2009, 01:25 +0200 schrieb Felix Zielcke: > Am Freitag, den 12.06.2009, 01:21 +0200 schrieb Felix Zielcke: > > Am Donnerstag, den 11.06.2009, 01:00 +0200 schrieb Felix Zielcke: > > > Am Dienstag, den 09.06.2009, 23:51 +0200 schrieb Vladimir 'phcoder' > > > Serbinenko: > > > > > > > + > > > > +char *grub_make_system_path_relative_to_its_root (char *path) > > > > +{ > > > > + > > > > + struct stat st; > > > > + char buf[500], buf2[500]; > > > > Use malloc instead of static allocation > > > > > > Changed. > > > > > > > + p = strrchr (buf, '/'); > > > > + if (p != buf) > > > > + *p = 0; > > > > + else *++p = 0; > > > > You assume path starts with /. You have to check this. Otherwise you > > > > may get sigsegv > > > > > > Changed. > > > > + strcpy(buf2,buf); > > > > Just save (p - buf) instead of copying buf to buf2 > > > > > > I did this now if realpath () isn't avaible. > > > Now realpath is used in case it's avaible. POSIX specifies it and > > > readlink is actually using it if it's avaible. I didn't read the source > > > correctly. > > > The problem is just Cygwin. It has it but it returns the cygwin path. > > > So C:\\Windows would get /cygdrive/c/windows, which is easy to handle. > > > But realpath ("/boot/grub") would return /boot/grub which isn't true > > > from Windows/GRUB point of view. > > > Maybe Christian and Bean can say something about it. > > > MingW doestn't have it and I don't know what Windows would have, so the > > > MingW users would still use my own way. > > > > Here's now a new one which aborts if realpath is avaible but doestn't > > support (path, NULL) > > and the fallback function is changed to dynamically allocate the memory. > > And now all memory is properly free'd > > args just noticed I forgot the *free_ptr = buf2; > I'm too lazy now to send an extra patch just for this. Ok here's a new one which compiles without warnings. -- Felix Zielcke [-- Attachment #2: make_sys_path_relative.patch.6 --] [-- Type: text/plain, Size: 4578 bytes --] 2009-06-12 Felix Zielcke <fzielcke@z-51.de> * configure.ac (AC_CHECK_FUNCS): Add realpath. * include/grub/util/hostdisk.c (grub_make_system_path_relative_to_its_root): New function prototype. * util/hostdisk.c: Include <stdint.h>. (grub_make_system_path_relative_to_its_root): New function. * util/i386/pc/grub-setup.c (setup): Use grub_make_system_path_relative_to_its_root to make core_path_dev relative to the partition. diff --git a/configure.ac b/configure.ac index b0f7bb7..9d594db 100644 --- a/configure.ac +++ b/configure.ac @@ -196,7 +196,7 @@ if test "$target_cpu"-"$platform" = i386-pc; then fi # Check for functions. -AC_CHECK_FUNCS(posix_memalign memalign asprintf) +AC_CHECK_FUNCS(posix_memalign memalign asprintf realpath) # # Check for target programs. diff --git a/include/grub/util/hostdisk.h b/include/grub/util/hostdisk.h index 21efb0d..5c648a1 100644 --- a/include/grub/util/hostdisk.h +++ b/include/grub/util/hostdisk.h @@ -23,5 +23,6 @@ void grub_util_biosdisk_init (const char *dev_map); void grub_util_biosdisk_fini (void); char *grub_util_biosdisk_get_grub_dev (const char *os_dev); +char *grub_make_system_path_relative_to_its_root (char *path, char **free_ptr); #endif /* ! GRUB_BIOSDISK_MACHINE_UTIL_HEADER */ diff --git a/util/hostdisk.c b/util/hostdisk.c index 1844a7e..25d01b2 100644 --- a/util/hostdisk.c +++ b/util/hostdisk.c @@ -27,6 +27,7 @@ #include <grub/misc.h> #include <stdio.h> +#include <stdint.h> #include <stdlib.h> #include <string.h> #include <ctype.h> @@ -1076,3 +1077,97 @@ grub_util_biosdisk_get_grub_dev (const char *os_dev) return make_device_name (drive, -1, -1); #endif } + +char * +grub_make_system_path_relative_to_its_root (char *path, char **free_ptr) +{ + struct stat st; + char *buf, *buf2; + uintptr_t offset = 0; + dev_t num; + size_t len; + char *p; +#ifndef HAVE_REALPATH + size_t len2; +#endif + + +#ifdef HAVE_REALPATH + p = realpath (path, NULL); + + if (p == NULL && errno != EINVAL) + grub_util_error ("failed to get realpath of %s", path); + else + grub_util_error ("realpath not supporting (path, NULL)"); + len = strlen (p) + 1; + buf = xmalloc (len); + buf2 = xmalloc (len); +# ifdef __CYGWIN__ + if (strncmp (p, 10, "/cygdrive/") == 0) + strcpy (buf, p + sizeof ("/cygdrive/c") - 1); + else + grub_util_error "path not under /cygdrive/"; +# else + strcpy (buf, p); +#endif + free (p); +#else /* ! HAVE_REALPATH */ +# warning "The function `grub_make_system_path_relative_to_its_root' might not work on your OS correctly." + if (*path != '/') + { + len2 = 4096; + do + { + p = getcwd (buf, len) + if (p == NULL) + { + if (errno != ERANGE) + grub_util_error ("can not get current working directory"); + else + len2 *= 2; + } + } while (p == NULL) + + buf = xmalloc (strlen (path) + len2 + 1); + strcat (buf, "/"); + strcat (buf, path); + } + else + { + buf = xmalloc (strlen (path) + 1); + strcpy (buf, path) + } +#endif /* ! HAVE_REALPATH */ + buf2 = xmalloc (strlen (buf) + 1); + strcpy (buf2, buf); + *free_ptr = buf2; + if (stat (buf, &st) < 0) + grub_util_error ("can not stat %s", p); + + num = st.st_dev; + while (1) + { + p = strrchr (buf, '/'); + if (p == NULL) + grub_util_error ("FIXME no / in buf"); + if (p != buf) + *p = 0; + else + *++p = 0; + + if (stat (buf, &st) < 0) + grub_util_error ("can not stat %s", buf); + + if (st.st_dev != num) + break; + + offset = p - buf; + if (offset == 1) + { + free (buf); + return buf2; + } + } + free (buf); + return buf2 + offset; +} diff --git a/util/i386/pc/grub-setup.c b/util/i386/pc/grub-setup.c index bdf234c..395db9d 100644 --- a/util/i386/pc/grub-setup.c +++ b/util/i386/pc/grub-setup.c @@ -89,7 +89,7 @@ setup (const char *dir, const char *root, const char *dest, int must_embed, int force) { char *boot_path, *core_path, *core_path_dev; - char *boot_img, *core_img; + char *boot_img, *core_img, *p, *free_ptr; size_t boot_size, core_size; grub_uint16_t core_sectors; grub_device_t root_dev, dest_dev; @@ -404,7 +404,10 @@ unable_to_embed: /* Make sure that GRUB reads the identical image as the OS. */ tmp_img = xmalloc (core_size); - core_path_dev = grub_util_get_path (dir, core_file); + p = grub_util_get_path (dir, core_file); + core_path_dev = grub_make_system_path_relative_to_its_root (p, &free_ptr); + free (p); + free (free_ptr); /* It is a Good Thing to sync two times. */ sync (); ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] Re: grub-install --root-directory=/mnt /dev/sda1 fails 2009-06-12 10:28 ` Felix Zielcke @ 2009-06-12 16:21 ` Pavel Roskin 2009-06-12 16:33 ` Felix Zielcke 0 siblings, 1 reply; 14+ messages in thread From: Pavel Roskin @ 2009-06-12 16:21 UTC (permalink / raw) To: The development of GRUB 2 On Fri, 2009-06-12 at 12:28 +0200, Felix Zielcke wrote: > Ok here's a new one which compiles without warnings. I suggest that whenever a patch is published, it comes with a detailed description. Surely, it could be gathered by rereading the thread, but I think it's not sufficient for two reasons. The first reason is that we want the patches reviewed by more people. Not everybody has time to read the whole discussion. The second reason is that the description the shows how the author sees the changes, what the potential benefits are, what code is affected. The reviewers would be able to compare the goals to the implementation. Even if the description was already published, it should not be hard to publish it again, perhaps with some improvements. Regarding this patch, I don't think we need to add a function to hostdisk.c is it's only used in grub-setup.c. It would be better to have it in grub-setup.c unless it's a generic function or there is a good chance that it will be used elsewhere. grub_make_system_path_relative_to_its_root is a very long name. There is nothing wrong with a long name per se, but it may be in sign that the function is too specialized and should not be in hostdisk.c. Following is not good for several reasons: #warning "The function `grub_make_system_path_relative_to_its_root' might not work on your OS correctly." The warning will only be seen by those who compile GRUB, but not by the end users who installed a binary. The warning doesn't explain what exactly may not work correctly. Since we are talking about new functionality here, I'd rather omit an unsafe implementation if realpath() is not available. What is free_ptr? It looks like it's a pointer that the caller should free. I think functions should be self-contained and should not ask the caller to do the cleanup (unless they actually return something useful in the allocated memory). -- Regards, Pavel Roskin ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Re: grub-install --root-directory=/mnt /dev/sda1 fails 2009-06-12 16:21 ` Pavel Roskin @ 2009-06-12 16:33 ` Felix Zielcke 0 siblings, 0 replies; 14+ messages in thread From: Felix Zielcke @ 2009-06-12 16:33 UTC (permalink / raw) To: The development of GRUB 2 Am Freitag, den 12.06.2009, 12:21 -0400 schrieb Pavel Roskin: > On Fri, 2009-06-12 at 12:28 +0200, Felix Zielcke wrote: > > > Ok here's a new one which compiles without warnings. > > I suggest that whenever a patch is published, it comes with a detailed > description. Surely, it could be gathered by rereading the thread, but > I think it's not sufficient for two reasons. > > The first reason is that we want the patches reviewed by more people. > Not everybody has time to read the whole discussion. > > The second reason is that the description the shows how the author sees > the changes, what the potential benefits are, what code is affected. > The reviewers would be able to compare the goals to the implementation. > > Even if the description was already published, it should not be hard to > publish it again, perhaps with some improvements. Well it does (well should) exactly do the same as the make_system_path_relative_to_its_root in util/grub-mkconfig_lib except that it's written in C instead of a bash function. > Regarding this patch, I don't think we need to add a function to > hostdisk.c is it's only used in grub-setup.c. It would be better to > have it in grub-setup.c unless it's a generic function or there is a > good chance that it will be used elsewhere. > > grub_make_system_path_relative_to_its_root is a very long name. There > is nothing wrong with a long name per se, but it may be in sign that the > function is too specialized and should not be in hostdisk.c. > > Following is not good for several reasons: > > #warning "The function `grub_make_system_path_relative_to_its_root' > might not work on your OS correctly." > > The warning will only be seen by those who compile GRUB, but not by the > end users who installed a binary. The warning doesn't explain what > exactly may not work correctly. Since we are talking about new > functionality here, I'd rather omit an unsafe implementation if > realpath() is not available. > > What is free_ptr? It looks like it's a pointer that the caller should > free. I think functions should be self-contained and should not ask the > caller to do the cleanup (unless they actually return something useful > in the allocated memory). Robert anyway already said that this needs discussing. The function is currently only useful in grub-setup in the case blocklists are used for core.img I don't think it will be useful for anything else. About free_ptr, yeah I could just copy buf2 + offset to a buf3 and then just free buf2 inside the function so that the caller can just free the return value of it. But I got this idea only after I sent the patch and I already told Vladimir that he doestn't need to look anymore because Robert wants to have the whole design discussed first. -- Felix Zielcke ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: grub-install --root-directory=/mnt /dev/sda1 fails 2009-06-01 19:39 ` Felix Zielcke 2009-06-08 21:20 ` [PATCH] " Felix Zielcke @ 2009-07-01 14:33 ` Felix Zielcke 2009-07-04 20:09 ` Robert Millan 1 sibling, 1 reply; 14+ messages in thread From: Felix Zielcke @ 2009-07-01 14:33 UTC (permalink / raw) To: The development of GRUB 2 Am Montag, den 01.06.2009, 21:39 +0200 schrieb Felix Zielcke: > Am Mittwoch, den 06.05.2009, 17:12 +0200 schrieb Vladimir 'phcoder' > Serbinenko: > > Don't we already have a function which transforms host directory into > > grub > > directory? AFAIR we have. > > There's just the shell function in grub-mkconfig_lib.in > Here's now a patch wich implements it in util/hostdisk.c and gets used > for core_path_dev in setup (). > But it doestn't work with symlinks. > readlink () can only be used if the file pointed to is a symlink, not if > a symlink is somewhere in between. > coreutils where the readlink binary is from is GPL 3+ but the function > for it uses hash tables and it seems like it would be too much code to > copy just for this. So what do we do know about this problem? We could just assume that the directory given with grub-setup --directory is already the real absolute path and just use the stat magic to make it relative. Or we use realpath() if avaible to get the real one. And on systems not having it, like mingw we could just assume that it's already relative and just return the path given. Or is there some other way this bug can be fixed? -- Felix Zielcke ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: grub-install --root-directory=/mnt /dev/sda1 fails 2009-07-01 14:33 ` Felix Zielcke @ 2009-07-04 20:09 ` Robert Millan 2009-07-08 14:16 ` Felix Zielcke 0 siblings, 1 reply; 14+ messages in thread From: Robert Millan @ 2009-07-04 20:09 UTC (permalink / raw) To: The development of GRUB 2 On Wed, Jul 01, 2009 at 04:33:25PM +0200, Felix Zielcke wrote: > Am Montag, den 01.06.2009, 21:39 +0200 schrieb Felix Zielcke: > > Am Mittwoch, den 06.05.2009, 17:12 +0200 schrieb Vladimir 'phcoder' > > Serbinenko: > > > Don't we already have a function which transforms host directory into > > > grub > > > directory? AFAIR we have. > > > > There's just the shell function in grub-mkconfig_lib.in > > Here's now a patch wich implements it in util/hostdisk.c and gets used > > for core_path_dev in setup (). > > But it doestn't work with symlinks. > > readlink () can only be used if the file pointed to is a symlink, not if > > a symlink is somewhere in between. > > coreutils where the readlink binary is from is GPL 3+ but the function > > for it uses hash tables and it seems like it would be too much code to > > copy just for this. > > So what do we do know about this problem? > We could just assume that the directory given with grub-setup > --directory is already the real absolute path and just use the stat > magic to make it relative. > Or we use realpath() if avaible to get the real one. > And on systems not having it, like mingw we could just assume that it's > already relative and just return the path given. > Or is there some other way this bug can be fixed? I just looked into this, and I don't think there's any real problem that needs fixing. The "workaround" is just a red herring, it works for him by pure chance. See: http://savannah.gnu.org/bugs/index.php?26924 -- 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." ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: grub-install --root-directory=/mnt /dev/sda1 fails 2009-07-04 20:09 ` Robert Millan @ 2009-07-08 14:16 ` Felix Zielcke 0 siblings, 0 replies; 14+ messages in thread From: Felix Zielcke @ 2009-07-08 14:16 UTC (permalink / raw) To: The development of GRUB 2 Am Samstag, den 04.07.2009, 22:09 +0200 schrieb Robert Millan: > On Wed, Jul 01, 2009 at 04:33:25PM +0200, Felix Zielcke wrote: > > Am Montag, den 01.06.2009, 21:39 +0200 schrieb Felix Zielcke: > > > Am Mittwoch, den 06.05.2009, 17:12 +0200 schrieb Vladimir 'phcoder' > > > Serbinenko: > > > > Don't we already have a function which transforms host directory into > > > > grub > > > > directory? AFAIR we have. > > > > > > There's just the shell function in grub-mkconfig_lib.in > > > Here's now a patch wich implements it in util/hostdisk.c and gets used > > > for core_path_dev in setup (). > > > But it doestn't work with symlinks. > > > readlink () can only be used if the file pointed to is a symlink, not if > > > a symlink is somewhere in between. > > > coreutils where the readlink binary is from is GPL 3+ but the function > > > for it uses hash tables and it seems like it would be too much code to > > > copy just for this. > > > > So what do we do know about this problem? > > We could just assume that the directory given with grub-setup > > --directory is already the real absolute path and just use the stat > > magic to make it relative. > > Or we use realpath() if avaible to get the real one. > > And on systems not having it, like mingw we could just assume that it's > > already relative and just return the path given. > > Or is there some other way this bug can be fixed? > > I just looked into this, and I don't think there's any real problem that > needs fixing. The "workaround" is just a red herring, it works for him by > pure chance. > > See: > > http://savannah.gnu.org/bugs/index.php?26924 I think I tested this and the root device was set correctly, so it looked at the right device for the core.img -- Felix Zielcke ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2009-07-08 14:16 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-05-06 14:31 grub-install --root-directory=/mnt /dev/sda1 fails Felix Zielcke 2009-05-06 15:12 ` Vladimir 'phcoder' Serbinenko 2009-06-01 19:39 ` Felix Zielcke 2009-06-08 21:20 ` [PATCH] " Felix Zielcke 2009-06-09 21:51 ` Vladimir 'phcoder' Serbinenko 2009-06-10 23:00 ` Felix Zielcke 2009-06-11 23:21 ` Felix Zielcke 2009-06-11 23:25 ` Felix Zielcke 2009-06-12 10:28 ` Felix Zielcke 2009-06-12 16:21 ` Pavel Roskin 2009-06-12 16:33 ` Felix Zielcke 2009-07-01 14:33 ` Felix Zielcke 2009-07-04 20:09 ` Robert Millan 2009-07-08 14:16 ` Felix Zielcke
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.