biosdisk, getroot for Cygwin

PATCH In review Christian Franke 2 participants 8 messages Nov 23, 2007 – May 16, 2008

lore

Share & tools

lore

Christian Franke

author

2007-11-23 20:30 UTC #0 lore raw

PATCH 1/1 biosdisk, getroot for Cygwin

This patch adds the functionality necessary for grub-probe on Cygwin.

It also fixes 2 bugs in strip_extra_slashes() and __GNU__ get_os_disk().

Christian

2007-11-23 Christian Franke <[email protected]>

* util/biosdisk.c: [__CYGWIN__] Add includes. (grub_util_biosdisk_open): Use Linux code also for Cygwin. (get_os_disk): Move variable declarations to OS specific parts to avoid warning.
[__GNU__] (get_os_disk): Fix /dev/sdXsN case. [__CYGWIN__] (get_os_disk): Add Cygwin /dev/sdXN device names. (grub_util_biosdisk_get_grub_dev): Use Linux code also for Cygwin.

* util/getroot.c: [__CYGWIN__] Add includes. (strip_extra_slashes): Fix "/" case.
[__CYGWIN__] (get_win32_path): New function. [__CYGWIN__] (grub_get_prefix): Add conversion to win32 path. [__CYGWIN__] (find_root_device): Disable. [__CYGWIN__] (get_bootsec_serial): New function. [__CYGWIN__] (find_cygwin_root_device): Likewise. [__CYGWIN__] (grub_util_get_grub_dev): Disable /dev/mapper check.

util/biosdisk.c +25 -9

diff -up grub2.orig/util/biosdisk.c grub2/util/biosdisk.c
@@ -76,6 +76,14 @@ struct hd_geometry
 # endif /* ! LOOP_MAJOR */
 #endif /* __linux__ */
 
+#ifdef __CYGWIN__
+# include <sys/ioctl.h>
+# include <cygwin/fs.h> /* BLKGETSIZE64 */
+# include <cygwin/hdreg.h> /* HDIO_GETGEO */
+# define MAJOR(dev)	((unsigned) ((dev) >> 16))
+# define FLOPPY_MAJOR	2
+#endif
+
 static char *map[256];
 
 #ifdef __linux__

@@ -161,7 +169,7 @@ grub_util_biosdisk_open (const char *nam
   disk->id = drive;
 
   /* Get the size.  */
-#ifdef __linux__
+#if defined(__linux__) || defined(__CYGWIN__)
   {
     unsigned long long nr;
     int fd;

@@ -595,16 +603,14 @@ make_device_name (int drive, int dos_par
 static char *
 get_os_disk (const char *os_dev)
 {
-  char *path, *p;
-  
 #if defined(__linux__)
-  path = xmalloc (PATH_MAX);
+  char *path = xmalloc (PATH_MAX);
   if (! realpath (os_dev, path))
     return 0;
   
   if (strncmp ("/dev/", path, 5) == 0)
     {
-      p = path + 5;
+      char *p = path + 5;
 
       if (have_devfs ())
 	{

@@ -654,15 +660,21 @@ get_os_disk (const char *os_dev)
   return path;
   
 #elif defined(__GNU__)
-  path = xstrdup (os_dev);
+  char *path = xstrdup (os_dev);
   if (strncmp ("/dev/sd", path, 7) == 0 || strncmp ("/dev/hd", path, 7) == 0)
     {
-      p = strchr (path, 's');
+      char *p = strrchr (path, 's');
       if (p)
 	*p = '\0';
     }
   return path;
 
+#elif defined(__CYGWIN__)
+  char *path = xstrdup (os_dev);
+  if (strncmp ("/dev/sd", path, 7) == 0 && 'a' <= path[7] && path[7] <= 'z')
+    path[8] = 0;
+  return path;
+
 #else
 # warning "The function `get_os_disk' might not work on your OS correctly."
   return xstrdup (os_dev);

@@ -713,11 +725,15 @@ grub_util_biosdisk_get_grub_dev (const c
   if (! S_ISBLK (st.st_mode))
     return make_device_name (drive, -1, -1);
   
-#if defined(__linux__)
+#if defined(__linux__) || defined(__CYGWIN__)
   /* Linux counts partitions uniformly, whether a BSD partition or a DOS
      partition, so mapping them to GRUB devices is not trivial.
      Here, get the start sector of a partition by HDIO_GETGEO, and
-     compare it with each partition GRUB recognizes.  */
+     compare it with each partition GRUB recognizes.
+
+     Cygwin /dev/sdXN emulation uses Windows partition mapping. It
+     does not count the extended partition and missing primary
+     partitions.  Use same method as on Linux here.  */
   {
     char *name;
     grub_disk_t disk;

util/getroot.c +137 -1

198 lines not on this page · raw message

Christian Franke

author

2008-05-07 20:42 UTC #1 lore raw

Updated version of the patch. It adds the functionality necessary for grub-probe on Cygwin.

It also fixes 2 bugs in strip_extra_slashes() and __GNU__ get_os_disk()

Christian

2008-05-07 Christian Franke <[email protected]>

util/biosdisk.c +25 -9

index e6e7f51..1b164a9 100644
@@ -77,6 +77,14 @@ struct hd_geometry
 # endif /* ! LOOP_MAJOR */
 #endif /* __linux__ */
 
+#ifdef __CYGWIN__
+# include <sys/ioctl.h>
+# include <cygwin/fs.h> /* BLKGETSIZE64 */
+# include <cygwin/hdreg.h> /* HDIO_GETGEO */
+# define MAJOR(dev)	((unsigned) ((dev) >> 16))
+# define FLOPPY_MAJOR	2
+#endif
+
 static char *map[256];
 
 #ifdef __linux__

@@ -162,7 +170,7 @@ grub_util_biosdisk_open (const char *name, grub_disk_t disk)
   disk->id = drive;
 
   /* Get the size.  */
-#ifdef __linux__
+#if defined(__linux__) || defined(__CYGWIN__)
   {
     unsigned long long nr;
     int fd;

@@ -603,16 +611,14 @@ make_device_name (int drive, int dos_part, int bsd_part)
 static char *
 get_os_disk (const char *os_dev)
 {
-  char *path, *p;
-  
 #if defined(__linux__)
-  path = xmalloc (PATH_MAX);
+  char *path = xmalloc (PATH_MAX);
   if (! realpath (os_dev, path))
     return 0;
   
   if (strncmp ("/dev/", path, 5) == 0)
     {
-      p = path + 5;
+      char *p = path + 5;
 
       /* If this is an IDE disk.  */
       if (strncmp ("ide/", p, 4) == 0)

@@ -659,15 +665,21 @@ get_os_disk (const char *os_dev)
   return path;
   
 #elif defined(__GNU__)
-  path = xstrdup (os_dev);
+  char *path = xstrdup (os_dev);
   if (strncmp ("/dev/sd", path, 7) == 0 || strncmp ("/dev/hd", path, 7) == 0)
     {
-      p = strchr (path, 's');
+      char *p = strchr (path + 7, 's');
       if (p)
 	*p = '\0';
     }
   return path;
 
+#elif defined(__CYGWIN__)
+  char *path = xstrdup (os_dev);
+  if (strncmp ("/dev/sd", path, 7) == 0 && 'a' <= path[7] && path[7] <= 'z')
+    path[8] = 0;
+  return path;
+
 #else
 # warning "The function `get_os_disk' might not work on your OS correctly."
   return xstrdup (os_dev);

@@ -718,11 +730,15 @@ grub_util_biosdisk_get_grub_dev (const char *os_dev)
   if (! S_ISBLK (st.st_mode))
     return make_device_name (drive, -1, -1);
   
-#if defined(__linux__)
+#if defined(__linux__) || defined(__CYGWIN__)
   /* Linux counts partitions uniformly, whether a BSD partition or a DOS
      partition, so mapping them to GRUB devices is not trivial.
      Here, get the start sector of a partition by HDIO_GETGEO, and
-     compare it with each partition GRUB recognizes.  */
+     compare it with each partition GRUB recognizes.
+
+     Cygwin /dev/sdXN emulation uses Windows partition mapping. It
+     does not count the extended partition and missing primary
+     partitions.  Use same method as on Linux here.  */
   {
     char *name;
     grub_disk_t disk;

util/getroot.c +140 -1

201 lines not on this page · raw message

Robert Millan

2008-05-09 12:46 UTC #2 lore raw

Quoted 4 lines · Christian Franke

util/getroot.c +4 -0

+
+  /* Check signature.  */
+  if (!(buf[0x1fe] == 0x55 && buf[0x1ff] == 0xaa))
+    return ~0;

util/getroot.c find_root_device (const char *dir, dev_t dev) · line 295

Quoted 10 lines

util/getroot.c +10 -0

+
+  /* Serial number offset depends on boot sector type.  */
+  if (mbr)
+    n = 0x1b8;
+  else if (memcmp (buf + 0x03, "NTFS", 4) == 0)
+    n = 0x048;
+  else if (memcmp (buf + 0x52, "FAT32", 5) == 0)
+    n = 0x043;
+  else if (memcmp (buf + 0x36, "FAT", 3) == 0)
+    n = 0x027;

Did you consider using a struct here?

util/getroot.c find_root_device (const char *dir, dev_t dev) · line 328

Quoted 5 lines

util/getroot.c +5 -0

+  /* Cygwin returns the partition serial number in stat.st_dev.
+     This is never identical to the device number of the emulated
+     /dev/sdXN device, so above find_root_device () does not work.
+     Search the partion with the same serial in boot sector instead.  */
+  char devpath[sizeof ("/dev/sda15") + 13];

Where does this 13 come from? Would be nice to make it explicit (e.g. sizeof(something) or so).

util/getroot.c grub_guess_root_device (const char *dir) · line 380

Quoted 13 lines

util/getroot.c +6 -0

+#ifndef __CYGWIN__
 #ifdef __linux__
   /* We first try to find the device in the /dev/mapper directory.  If
      we don't do this, we get useless device names like /dev/dm-0 for

@@ -242,12 +373,19 @@ grub_guess_root_device (const char *dir)
       os_dev = find_root_device ("/dev", st.st_dev);
     }
 
+#else
+  /* Cygwin specific function.  */
+  os_dev = find_cygwin_root_device (dir, st.st_dev);
+
+#endif /* __CYGWIN__ */

The logic here seems a bit over-complicated. Surely whenever you have __linux__ you don't have __CYGWIN__. Are you sure it can't be simplified?

util/getroot.c grub_util_get_dev_abstraction (const char *os_dev) · line 396

Quoted 9 lines

util/getroot.c +2 -0

+#ifndef __CYGWIN__
   /* Check for LVM.  */
   if (!strncmp (os_dev, "/dev/mapper/", 12))
     return GRUB_DEV_ABSTRACTION_LVM;

@@ -255,6 +393,7 @@ grub_util_get_dev_abstraction (const char *os_dev)
   /* Check for RAID.  */
   if (!strncmp (os_dev, "/dev/md", 7))
     return GRUB_DEV_ABSTRACTION_RAID;
+#endif /* !__CYGWIN__ */

I think what we're missing here is "ifdef __linux__" since both LVM and dmRAID are Linux-specific.

Signature · 5 lines

Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Christian Franke

author

2008-05-09 17:32 UTC #3 lore raw

Quoted 11 lines · Robert Millan

Quoted 9 lines · Christian Franke

util/getroot.c +9 -0

+
+  /* Check signature.  */
+  if (!(buf[0x1fe] == 0x55 && buf[0x1ff] == 0xaa))
+    return ~0;
+
+  /* Serial number offset depends on boot sector type.  */
+  if (mbr)
+    n = 0x1b8;
+  else if (memcmp (buf + 0x03, "NTFS", 4) == 0)

Quoted 10 lines

util/getroot.c find_root_device (const char *dir, dev_t dev) · line 295
Quoted 6 lines
util/getroot.c +5 -0
+    n = 0x048;
+  else if (memcmp (buf + 0x52, "FAT32", 5) == 0)
+    n = 0x043;
+  else if (memcmp (buf + 0x36, "FAT", 3) == 0)
+    n = 0x027;
Did you consider using a struct here?

There are 4 boot sector types, a clean declaration would require a union of 4 structs. IMO not worth the effort.

Quoted 11 lines

util/getroot.c find_root_device (const char *dir, dev_t dev) · line 328
Quoted 6 lines
util/getroot.c +5 -0
+  /* Cygwin returns the partition serial number in stat.st_dev.
+     This is never identical to the device number of the emulated
+     /dev/sdXN device, so above find_root_device () does not work.
+     Search the partion with the same serial in boot sector instead.  */
+  char devpath[sizeof ("/dev/sda15") + 13];
Where does this 13 come from? Would be nice to make it explicit (e.g. sizeof(something) or so).

13 "paranoia" bytes added to the required size :-)

Quoted 8 lines

util/getroot.c +1 -0

+#ifndef __CYGWIN__
 #ifdef __linux__
   /* We first try to find the device in the /dev/mapper directory.  If
      we don't do this, we get useless device names like /dev/dm-0 for

@@ -242,12 +373,19 @@ grub_guess_root_device (const char *dir)
       os_dev = find_root_device ("/dev", st.st_dev);
     }

Quoted 11 lines

util/getroot.c grub_guess_root_device (const char *dir) · line 384
Quoted 6 lines
util/getroot.c +5 -0
+#else
+  /* Cygwin specific function.  */
+  os_dev = find_cygwin_root_device (dir, st.st_dev);
+
+#endif /* __CYGWIN__ */
 
The logic here seems a bit over-complicated. Surely whenever you have __linux__ you don't have __CYGWIN__. Are you sure it can't be simplified?

An alternative would be to move the #ifndef __CYGWIN__ behind the #ifdef __linux__ block:

#ifdef __linux__
...
os_dev = find_root_device ("/dev/mapper", st.st_dev);
...
if (! os_dev)
#endif
#ifndef __CYGWIN__

   {
     /* This might be truly slow, but is there any better way?  */
     os_dev = find_root_device ("/dev", st.st_dev);

}

#else /* __CYGWIN__ */
/* Cygwin specific function. */
os_dev = find_cygwin_root_device (dir, st.st_dev);

#endif /* __CYGWIN__ */

I'm not sure whether this would be more readable.

Quoted 5 lines

util/getroot.c +1 -0

+#ifndef __CYGWIN__
   /* Check for LVM.  */
   if (!strncmp (os_dev, "/dev/mapper/", 12))
     return GRUB_DEV_ABSTRACTION_LVM;

@@ -255,6 +393,7 @@ grub_util_get_dev_abstraction (const char *os_dev)

Quoted 10 lines

util/getroot.c grub_util_get_dev_abstraction (const char *os_dev) · line 397
Quoted 5 lines
util/getroot.c +1 -0
   /* Check for RAID.  */
   if (!strncmp (os_dev, "/dev/md", 7))
     return GRUB_DEV_ABSTRACTION_RAID;
+#endif /* !__CYGWIN__ */
 
I think what we're missing here is "ifdef __linux__" since both LVM and dmRAID are Linux-specific.

Yes.

Christian

Christian Franke

author

2008-05-11 21:04 UTC #4 lore raw

Quoted 36 lines · Christian Franke

Quoted 9 lines · Robert Millan
Quoted 8 lines · Christian Franke
util/getroot.c +1 -0
+#ifndef __CYGWIN__
 #ifdef __linux__
   /* We first try to find the device in the /dev/mapper directory.  If
      we don't do this, we get useless device names like /dev/dm-0 for
@@ -242,12 +373,19 @@ grub_guess_root_device (const char *dir)
       os_dev = find_root_device ("/dev", st.st_dev);
     }
 
Quoted 11 lines
util/getroot.c grub_guess_root_device (const char *dir) · line 384
Quoted 6 lines
util/getroot.c +5 -0
+#else
+  /* Cygwin specific function.  */
+  os_dev = find_cygwin_root_device (dir, st.st_dev);
+
+#endif /* __CYGWIN__ */
 
The logic here seems a bit over-complicated. Surely whenever you have __linux__ you don't have __CYGWIN__. Are you sure it can't be simplified?
An alternative would be to move the #ifndef __CYGWIN__ behind the #ifdef __linux__ block:
#ifdef __linux__
...
os_dev = find_root_device ("/dev/mapper", st.st_dev);
...
if (! os_dev)
#endif
#ifndef __CYGWIN__
   {
     /* This might be truly slow, but is there any better way?  */
     os_dev = find_root_device ("/dev", st.st_dev);
   }

Quoted 9 lines

#else /* __CYGWIN__ */
/* Cygwin specific function. */
os_dev = find_cygwin_root_device (dir, st.st_dev);
#endif /* __CYGWIN__ */
I'm not sure whether this would be more readable.

Probably more readable and extensible - Use early returns:

#ifdef __linux__
/* We first try to find the device in the /dev/mapper directory. If
we don't do this, we get useless device names like /dev/dm-0 for LVM. */
os_dev = find_root_device ("/dev/mapper", st.st_dev);
if (os_dev)
return os_dev;

/* The same applies to /dev/evms directory (for EVMS volumes). */ os_dev = find_root_device ("/dev/evms", st.st_dev);
if (os_dev)
return os_dev;
#endif /* __linux__ */

#ifndef __CYGWIN__
/* This might be truly slow, but is there any better way? */ os_dev = find_root_device ("/dev", st.st_dev);

#else /* __CYGWIN__ */
/* Cygwin specific function. */
os_dev = find_cygwin_root_device (dir, st.st_dev);

#endif /* __CYGWIN__ */

return os_dev;
}

Christian

Robert Millan

2008-05-12 15:26 UTC #5 lore raw

In reply to Christian Franke

Quoted 7 lines · Christian Franke

Quoted 5 lines
Quoted 2 lines
+ char devpath[sizeof ("/dev/sda15") + 13];
Where does this 13 come from? Would be nice to make it explicit (e.g. sizeof(something) or so).
13 "paranoia" bytes added to the required size :-)

A comment saying that would be nice.

Quoted 2 lines · Christian Franke

Probably more readable and extensible - Use early returns:

Yeah I like that better too.

Quoted 9 lines

#ifndef __CYGWIN__
/* This might be truly slow, but is there any better way? */ os_dev = find_root_device ("/dev", st.st_dev);
#else /* __CYGWIN__ */
/* Cygwin specific function. */
os_dev = find_cygwin_root_device (dir, st.st_dev);
#endif /* __CYGWIN__ */

I'd suggest removing the double-negation here; it looks more readable with "#ifdef / #else" IMHO (and is easier to add more OS-specific checks like "#if defined(__CYGWIN__) / #elif defined(...) / #else".

Signature · 5 lines

Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Christian Franke

author

2008-05-13 21:18 UTC #6 lore raw

Quoted 5 lines · Robert Millan

Quoted 3 lines · Christian Franke
Quoted 3 lines
Quoted 3 lines
+ char devpath[sizeof ("/dev/sda15") + 13];

Quoted 9 lines

Quoted 5 lines
Quoted 3 lines
Where does this 13 come from? Would be nice to make it explicit (e.g. sizeof(something) or so).
13 "paranoia" bytes added to the required size :-)
A comment saying that would be nice.

Done.

Quoted 16 lines · Christian Franke

Quoted 3 lines
Probably more readable and extensible - Use early returns:
Yeah I like that better too.
Quoted 8 lines
#ifndef __CYGWIN__
/* This might be truly slow, but is there any better way? */ os_dev = find_root_device ("/dev", st.st_dev);
#else /* __CYGWIN__ */
/* Cygwin specific function. */
os_dev = find_cygwin_root_device (dir, st.st_dev);

Quoted 8 lines

Quoted 2 lines
#endif /* __CYGWIN__ */
I'd suggest removing the double-negation here; it looks more readable with "#ifdef / #else" IMHO (and is easier to add more OS-specific checks like "#if defined(__CYGWIN__) / #elif defined(...) / #else".

Done, new patch below. If there are no further comments, I will commit it in a few days.

Christian

2008-05-13 Christian Franke <[email protected]>

* util/getroot.c: [__CYGWIN__] Add includes. (strip_extra_slashes): Fix "/" case.
[__CYGWIN__] (get_win32_path): New function. [__CYGWIN__] (grub_get_prefix): Add conversion to win32 path. [__CYGWIN__] (find_root_device): Disable.
[__CYGWIN__] (get_bootsec_serial): New function. [__CYGWIN__] (find_cygwin_root_device): Likewise. [__linux__] (grub_guess_root_device): Add early returns to simplify structure.
[__CYGWIN__] (grub_guess_root_device): Call find_cygwin_root_device. [__linux__] (grub_util_get_dev_abstraction): Enable LVM and RAID check for Linux only.

util/biosdisk.c +25 -9

index fcf01a4..ddb3718 100644
@@ -77,6 +77,14 @@ struct hd_geometry
 # endif /* ! LOOP_MAJOR */
 #endif /* __linux__ */
 
+#ifdef __CYGWIN__
+# include <sys/ioctl.h>
+# include <cygwin/fs.h> /* BLKGETSIZE64 */
+# include <cygwin/hdreg.h> /* HDIO_GETGEO */
+# define MAJOR(dev)	((unsigned) ((dev) >> 16))
+# define FLOPPY_MAJOR	2
+#endif
+
 static char *map[256];
 
 #ifdef __linux__

@@ -162,7 +170,7 @@ grub_util_biosdisk_open (const char *name, grub_disk_t disk)
   disk->id = drive;
 
   /* Get the size.  */
-#ifdef __linux__
+#if defined(__linux__) || defined(__CYGWIN__)
   {
     unsigned long long nr;
     int fd;

@@ -616,16 +624,14 @@ make_device_name (int drive, int dos_part, int bsd_part)
 static char *
 get_os_disk (const char *os_dev)
 {
-  char *path, *p;
-  
 #if defined(__linux__)
-  path = xmalloc (PATH_MAX);
+  char *path = xmalloc (PATH_MAX);
   if (! realpath (os_dev, path))
     return 0;
   
   if (strncmp ("/dev/", path, 5) == 0)
     {
-      p = path + 5;
+      char *p = path + 5;
 
       /* If this is an IDE disk.  */
       if (strncmp ("ide/", p, 4) == 0)

@@ -692,15 +698,21 @@ get_os_disk (const char *os_dev)
   return path;
   
 #elif defined(__GNU__)
-  path = xstrdup (os_dev);
+  char *path = xstrdup (os_dev);
   if (strncmp ("/dev/sd", path, 7) == 0 || strncmp ("/dev/hd", path, 7) == 0)
     {
-      p = strchr (path, 's');
+      char *p = strchr (path + 7, 's');
       if (p)
 	*p = '\0';
     }
   return path;
 
+#elif defined(__CYGWIN__)
+  char *path = xstrdup (os_dev);
+  if (strncmp ("/dev/sd", path, 7) == 0 && 'a' <= path[7] && path[7] <= 'z')
+    path[8] = 0;
+  return path;
+
 #else
 # warning "The function `get_os_disk' might not work on your OS correctly."
   return xstrdup (os_dev);

@@ -751,11 +763,15 @@ grub_util_biosdisk_get_grub_dev (const char *os_dev)
   if (! S_ISBLK (st.st_mode))
     return make_device_name (drive, -1, -1);
   
-#if defined(__linux__)
+#if defined(__linux__) || defined(__CYGWIN__)
   /* Linux counts partitions uniformly, whether a BSD partition or a DOS
      partition, so mapping them to GRUB devices is not trivial.
      Here, get the start sector of a partition by HDIO_GETGEO, and
-     compare it with each partition GRUB recognizes.  */
+     compare it with each partition GRUB recognizes.
+
+     Cygwin /dev/sdXN emulation uses Windows partition mapping. It
+     does not count the extended partition and missing primary
+     partitions.  Use same method as on Linux here.  */
   {
     char *name;
     grub_disk_t disk;

util/getroot.c +150 -8

216 lines not on this page · raw message

Christian Franke

author

2008-05-16 21:43 UTC #7 lore raw

Quoted 4 lines

Done, new patch below. If there are no further comments, I will commit it in a few days.

Committed.

Christian