mkdevicemap for Cygwin

PATCH In review Christian Franke 3 participants 14 messages Nov 13, 2007 – May 5, 2008

lore

Share & tools

lore

Christian Franke

author

2007-11-13 20:42 UTC #0 lore raw

PATCH 1/1 mkdevicemap for Cygwin

Some changes for grub-mkdevicemap on Cygwin.

Christian

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

* util/grub-mkdevicemap.c (get_floppy_disk_name): Add Cygwin device names.
(get_ide_disk_name): Disable on __CYGWIN__. (get_scsi_disk_name): Add Cygwin device names. (check_device): Add static.
Return error instead of success on empty string. (make_device_map): Disable IDE loop on __CYGWIN__. Move label inside linux specific code to prevent compiler warning.

util/grub-mkdevicemap.c +17 -6

@@ -166,6 +166,9 @@ get_floppy_disk_name (char *name, int un
 #elif defined(__QNXNTO__)
   /* QNX RTP */
   sprintf (name, "/dev/fd%d", unit);
+#elif defined(__CYGWIN__)
+  /* Cygwin */
+  sprintf (name, "/dev/fd%d", unit);
 #else
 # warning "BIOS floppy drives cannot be guessed in your operating system."
   /* Set NAME to a bogus string.  */

@@ -173,6 +176,7 @@ get_floppy_disk_name (char *name, int un
 #endif
 }
 
+#ifndef __CYGWIN__
 static void
 get_ide_disk_name (char *name, int unit)
 {

@@ -213,6 +217,7 @@ get_ide_disk_name (char *name, int unit)
   *name = 0;
 #endif
 }
+#endif /* __CYGWIN__ */
 
 static void
 get_scsi_disk_name (char *name, int unit)

@@ -248,6 +253,9 @@ get_scsi_disk_name (char *name, int unit
   /* QNX RTP doesn't distinguish SCSI from IDE, so it is better to
      disable the detection of SCSI disks here.  */
   *name = 0;
+#elif defined(__CYGWIN__)
+  /* Cygwin emulates all disks as /dev/sdX.  */
+  sprintf (name, "/dev/sd%c", unit + 'a');
 #else
 # warning "BIOS SCSI drives cannot be guessed in your operating system."
   /* Set NAME to a bogus string.  */

@@ -277,16 +285,16 @@ get_i2o_disk_name (char *name, char unit
 
 /* Check if DEVICE can be read. If an error occurs, return zero,
    otherwise return non-zero.  */
-int
+static int
 check_device (const char *device)
 {
   char buf[512];
   FILE *fp;
 
-  /* If DEVICE is empty, just return 1.  */
+  /* If DEVICE is empty, just return error.  */
   if (*device == 0)
-    return 1;
-  
+    return 0;
+ 
   fp = fopen (device, "r");
   if (! fp)
     {

@@ -415,7 +423,8 @@ make_device_map (const char *device_map,
       goto finish;
     }
 #endif /* __linux__ */
-    
+
+#ifndef __CYGWIN__    
   /* IDE disks.  */
   for (i = 0; i < 8; i++)
     {

@@ -431,6 +440,7 @@ make_device_map (const char *device_map,
 	  num_hd++;
 	}
     }
+#endif /* __CYGWIN__ */
   
 #ifdef __linux__
   /* ATARAID disks.  */

@@ -513,9 +523,10 @@ make_device_map (const char *device_map,
 	  }
       }
   }
-#endif /* __linux__ */
 
  finish:
+#endif /* __linux__ */
+
   if (fp != stdout)
     fclose (fp);
 }

Robert Millan

2007-11-18 06:24 UTC #1 lore raw

util/grub-mkdevicemap.c get_floppy_disk_name (char *name, int un · line 171

Quoted 3 lines · Christian Franke

util/grub-mkdevicemap.c +3 -0

+#elif defined(__CYGWIN__)
+  /* Cygwin */
+  sprintf (name, "/dev/fd%d", unit);

Cygwin has /dev now? :-)

Quoted 17 lines

util/grub-mkdevicemap.c +3 -0

+#ifndef __CYGWIN__
 static void
 get_ide_disk_name (char *name, int unit)
 {

@@ -213,6 +217,7 @@ get_ide_disk_name (char *name, int unit)
   *name = 0;
 #endif
 }
+#endif /* __CYGWIN__ */
 
[...]
 
+#ifndef __CYGWIN__    
   /* IDE disks.  */
   for (i = 0; i < 8; i++)
     {

util/grub-mkdevicemap.c make_device_map (const char *device_map, · line 443

Quoted 4 lines

util/grub-mkdevicemap.c +1 -0

@@ -431,6 +440,7 @@ make_device_map (const char *device_map,
 	  num_hd++;
 	}
     }
+#endif /* __CYGWIN__ */

Is the generic case (`*name = 0', then print that) good enough?

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

2007-11-18 18:40 UTC #2 lore raw

Quoted 10 lines · Robert Millan

util/grub-mkdevicemap.c get_floppy_disk_name (char *name, int un · line 171
Quoted 4 lines · Christian Franke
util/grub-mkdevicemap.c +3 -0
+#elif defined(__CYGWIN__)
+  /* Cygwin */
+  sprintf (name, "/dev/fd%d", unit);
Cygwin has /dev now? :-)

At least since 2003.

Quoted 16 lines

Quoted 14 lines

util/grub-mkdevicemap.c +2 -0

+#ifndef __CYGWIN__
 static void
 get_ide_disk_name (char *name, int unit)
 {

@@ -213,6 +217,7 @@ get_ide_disk_name (char *name, int unit)
   *name = 0;
 #endif
 }
+#endif /* __CYGWIN__ */
 
 
 
[...]

Quoted 14 lines

util/grub-mkdevicemap.c make_device_map (const char *device_map, · line 444
Quoted 10 lines
util/grub-mkdevicemap.c +2 -0
+#ifndef __CYGWIN__    
   /* IDE disks.  */
   for (i = 0; i < 8; i++)
     {
@@ -431,6 +440,7 @@ make_device_map (const char *device_map,
 	  num_hd++;
 	}
     }
+#endif /* __CYGWIN__ */
 
Is the generic case (`*name = 0', then print that) good enough?

No, it did not work because check_device() returned 1 ("exists") on (*name == 0). This results in 8 bogus "(hd N)\t\n" lines and wrong N for the real devices.

Yes, it would work now, because the patch also fixes this bug :-)

Christian

Marco Gerards

2007-11-18 11:31 UTC #3 lore raw

In reply to Christian Franke

Quoted 9 lines · Christian Franke

Some changes for grub-mkdevicemap on Cygwin.
Christian
2007-11-13 Christian Franke <[email protected]>
	* util/grub-mkdevicemap.c (get_floppy_disk_name): Add Cygwin
	device names.
	(get_ide_disk_name): Disable on __CYGWIN__.

        [__CYGWIN__] (get_ide_disk_name): Disable.

Quoted 3 lines

(get_scsi_disk_name): Add Cygwin device names. (check_device): Add static.
Return error instead of success on empty string.

Can you explain this? Does this break something on GNU/Linux?

Quoted 1 line

(make_device_map): Disable IDE loop on __CYGWIN__.

[__CYGWIN__] (make_device_map): Disable IDE loop.

Quoted 1 line

Move label inside linux specific code to prevent compiler warning.

(make_device_map): Move label inside linux specific code to prevent compiler warning.

Otherwise fine, so this can be committed if there aren't any other problems with this patch.

--
Marco

Christian Franke

author

2007-11-18 18:47 UTC #4 lore raw

Quoted 17 lines · Marco Gerards

Quoted 10 lines · Christian Franke
Some changes for grub-mkdevicemap on Cygwin.
Christian
2007-11-13 Christian Franke <[email protected]>
	* util/grub-mkdevicemap.c (get_floppy_disk_name): Add Cygwin
	device names.
	(get_ide_disk_name): Disable on __CYGWIN__.
        [__CYGWIN__] (get_ide_disk_name): Disable.

Quoted 8 lines

Quoted 4 lines
(get_scsi_disk_name): Add Cygwin device names. (check_device): Add static.
Return error instead of success on empty string.
Can you explain this? Does this break something on GNU/Linux?

See last mail. Should not break GNU/Linux, because (*name = 0) is not used then.

Quoted 6 lines

Quoted 2 lines
(make_device_map): Disable IDE loop on __CYGWIN__.
[__CYGWIN__] (make_device_map): Disable IDE loop.

Quoted 9 lines

Quoted 2 lines
Move label inside linux specific code to prevent compiler warning.
(make_device_map): Move label inside linux specific code to prevent compiler warning.
Otherwise fine, so this can be committed if there aren't any other problems with this patch.

Thanks, new changelog below.

Christian

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

* util/grub-mkdevicemap.c (get_floppy_disk_name): Add Cygwin device names.
[__CYGWIN__] (get_ide_disk_name): Disable. (get_scsi_disk_name): Add Cygwin device names. (check_device): Add static.
Return error instead of success on empty string. [__CYGWIN__] (make_device_map): Disable IDE loop. (make_device_map): Move label inside linux specific code to prevent compiler warning.

Robert Millan

2007-12-06 12:15 UTC #5 lore raw

In reply to Christian Franke

Quoted 6 lines · Christian Franke

No, it did not work because check_device() returned 1 ("exists") on (*name == 0). This results in 8 bogus "(hd N)\t\n" lines and wrong N for the real devices.
Yes, it would work now, because the patch also fixes this bug :-)

Ok, so in that case you don't need...

Quoted 16 lines

util/grub-mkdevicemap.c +3 -0

+#ifndef __CYGWIN__
 static void
 get_ide_disk_name (char *name, int unit)
 {

@@ -213,6 +217,7 @@ get_ide_disk_name (char *name, int unit)
   *name = 0;
 #endif
 }
+#endif /* __CYGWIN__ */
 
[...]
+#ifndef __CYGWIN__    
   /* IDE disks.  */
   for (i = 0; i < 8; i++)
     {

util/grub-mkdevicemap.c make_device_map (const char *device_map, · line 443

Quoted 4 lines

util/grub-mkdevicemap.c +1 -0

@@ -431,6 +440,7 @@ make_device_map (const char *device_map,
 	  num_hd++;
 	}
     }
+#endif /* __CYGWIN__ */

...because you already fixed the problem in:

util/grub-mkdevicemap.c get_i2o_disk_name (char *name, char unit · line 296

Quoted 6 lines

util/grub-mkdevicemap.c +2 -3

-  /* If DEVICE is empty, just return 1.  */
+  /* If DEVICE is empty, just return error.  */
   if (*device == 0)
-    return 1;
-  
+    return 0;

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

2007-12-06 13:09 UTC #6 lore raw

Quoted 28 lines · Robert Millan

Quoted 2 lines
...
Yes, it would work now, because the patch also fixes this bug :-)
Ok, so in that case you don't need...
Quoted 16 lines
+#ifndef __CYGWIN__
static void
get_ide_disk_name (char *name, int unit)
{
util/grub-mkdevicemap.c +2 -0
@@ -213,6 +217,7 @@ get_ide_disk_name (char *name, int unit)
 *name = 0;
 #endif
 }
+#endif /* __CYGWIN__ */
 
[...]
+#ifndef __CYGWIN__
/* IDE disks. */
for (i = 0; i < 8; i++)
{
util/grub-mkdevicemap.c make_device_map (const char *device_map, · line 443
Quoted 4 lines
util/grub-mkdevicemap.c +1 -0
@@ -431,6 +440,7 @@ make_device_map (const char *device_map,
 num_hd++;
 }
 }
+#endif /* __CYGWIN__ */
...because you already fixed the problem in:

Quoted 8 lines

util/grub-mkdevicemap.c get_i2o_disk_name (char *name, char unit · line 296
Quoted 6 lines
util/grub-mkdevicemap.c +2 -3
-  /* If DEVICE is empty, just return 1.  */
+  /* If DEVICE is empty, just return error.  */
 if (*device == 0)
-    return 1;
-
+    return 0;
?

Yes. I you want this alternative instead, and there are no other issues with this patch, I'm happy to post a new version.

Christian

Christian Franke

author

2007-12-28 15:43 UTC #7 lore raw

In reply to Robert Millan

Quoted 30 lines · Robert Millan

Quoted 6 lines · Christian Franke
No, it did not work because check_device() returned 1 ("exists") on (*name == 0). This results in 8 bogus "(hd N)\t\n" lines and wrong N for the real devices.
Yes, it would work now, because the patch also fixes this bug :-)
Ok, so in that case you don't need...
Review
Quoted 6 lines
(quoted diff) +3 -0
+#ifndef __CYGWIN__
...
+#ifndef __CYGWIN__    
...
+#endif /* __CYGWIN__ */
...because you already fixed the problem in:
Review
Quoted 7 lines
(quoted diff) +2 -3
-  /* If DEVICE is empty, just return 1.  */
+  /* If DEVICE is empty, just return error.  */
   if (*device == 0)
-    return 1;
-  
+    return 0;

Quoted 2 lines

?

New version of the patch is attached.

Christian

2007-12-28 Christian Franke <[email protected]>

* util/grub-mkdevicemap.c (get_floppy_disk_name) [__CYGWIN__]:
Add Cygwin device names.
(get_ide_disk_name) [__CYGWIN__]: Likewise. (get_scsi_disk_name) [__CYGWIN__]: Likewise. (check_device): Add static.
Return error instead of success on empty name. (make_device_map): Move label inside linux specific code to prevent compiler warning.

util/grub-mkdevicemap.c +15 -4

@@ -166,6 +166,9 @@ get_floppy_disk_name (char *name, int un
 #elif defined(__QNXNTO__)
   /* QNX RTP */
   sprintf (name, "/dev/fd%d", unit);
+#elif defined(__CYGWIN__)
+  /* Cygwin */
+  sprintf (name, "/dev/fd%d", unit);
 #else
 # warning "BIOS floppy drives cannot be guessed in your operating system."
   /* Set NAME to a bogus string.  */

@@ -207,6 +210,10 @@ get_ide_disk_name (char *name, int unit)
   /* Actually, QNX RTP doesn't distinguish IDE from SCSI, so this could
      contain SCSI disks.  */
   sprintf (name, "/dev/hd%d", unit);
+#elif defined(__CYGWIN__)
+  /* Cygwin emulates all disks as /dev/sdX.  */
+  (void) unit;
+  *name = 0;
 #else
 # warning "BIOS IDE drives cannot be guessed in your operating system."
   /* Set NAME to a bogus string.  */

@@ -248,6 +255,9 @@ get_scsi_disk_name (char *name, int unit
   /* QNX RTP doesn't distinguish SCSI from IDE, so it is better to
      disable the detection of SCSI disks here.  */
   *name = 0;
+#elif defined(__CYGWIN__)
+  /* Cygwin emulates all disks as /dev/sdX.  */
+  sprintf (name, "/dev/sd%c", unit + 'a');
 #else
 # warning "BIOS SCSI drives cannot be guessed in your operating system."
   /* Set NAME to a bogus string.  */

@@ -277,15 +287,15 @@ get_i2o_disk_name (char *name, char unit
 
 /* Check if DEVICE can be read. If an error occurs, return zero,
    otherwise return non-zero.  */
-int
+static int
 check_device (const char *device)
 {
   char buf[512];
   FILE *fp;
 
-  /* If DEVICE is empty, just return 1.  */
+  /* If DEVICE is empty, just return error.  */
   if (*device == 0)
-    return 1;
+    return 0;
   
   fp = fopen (device, "r");
   if (! fp)

@@ -513,9 +523,10 @@ make_device_map (const char *device_map,
 	  }
       }
   }
-#endif /* __linux__ */
 
  finish:
+#endif /* __linux__ */
+
   if (fp != stdout)
     fclose (fp);
 }

Marco Gerards

2008-01-23 11:39 UTC #8 lore raw

Quoted 1 line · Christian Franke

New version of the patch is attached.

[...]

Robert, did you review it again?

--
Marco

Robert Millan

2008-01-23 11:47 UTC #9 lore raw

Quoted 7 lines · Marco Gerards

Quoted 1 line · Christian Franke
New version of the patch is attached.
[...]
Robert, did you review it again?

It looks fine to me.

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 /.)

Marco Gerards

2008-01-23 12:21 UTC #10 lore raw

Quoted 10 lines · Robert Millan

Quoted 7 lines · Marco Gerards
Quoted 1 line · Christian Franke
New version of the patch is attached.
[...]
Robert, did you review it again?
It looks fine to me.

As far as I am concerned it can be committed. We can even add Christian to savannah so he can commit it.

--
Marco

Christian Franke

author

2008-01-23 20:41 UTC #11 lore raw

Quoted 10 lines · Marco Gerards

Quoted 3 lines · Robert Millan
It looks fine to me.
As far as I am concerned it can be committed. We can even add Christian to savannah so he can commit it.

Thanks, my savannah account is chrfranke.

Christian

Marco Gerards

2008-04-24 20:07 UTC #12 lore raw

Quoted 5 lines · Christian Franke

Quoted 3 lines · Marco Gerards
Quoted 1 line · Robert Millan

Quoted 8 lines

Quoted 6 lines
Quoted 2 lines
It looks fine to me.
As far as I am concerned it can be committed. We can even add Christian to savannah so he can commit it.
Thanks, my savannah account is chrfranke.

Sorry for the *very* long delay. I rarely check the list nowadays and forgot all about this.

Anyways, you have been added.

--
Marco

Christian Franke

author

2008-05-05 21:27 UTC #13 lore raw

Quoted 16 lines · Marco Gerards

Quoted 13 lines · Christian Franke
Quoted 10 lines · Marco Gerards
Quoted 3 lines · Robert Millan
It looks fine to me.
As far as I am concerned it can be committed. We can even add Christian to savannah so he can commit it.

Quoted 7 lines

Quoted 2 lines
Thanks, my savannah account is chrfranke.
Sorry for the *very* long delay. I rarely check the list nowadays and forgot all about this.

No problem.

Quoted 3 lines

Anyways, you have been added.

Thanks.

Committed.

Christian