[syslinux] [PATCH] Mending proposals for mkdiskimage

[Thomas Schmitt]

Hi,

this is the diff which i consider a halfways reasonable solution
for the following problems with mkdiskimage:

1. Ignoring option -s if the target file is not suitable for truncate().

2. Miscalculation of truncate() size by a factor of 512.

3. The storage capacity of device files can only be measured
   for Linux block devices.

4. Long time of silent work when slow devices (e.g. USB sticks) get
   formatted without option -s.

Another issue is not necessarily a problem but might leave the
possibility of confusing firmware by e.g. an outdated GPT backup: 

5. No zeroizing of eventual past-partition space.
   Such space remains e.g. if the given number of megabytes or the
   determined size of the target device is not aligned to cylinder
   size. (It also remains if the device or existing file size is
   larger than an explicitely given number of cylinders.)
  
-----------------------------------------------------------------

The implemented solutions are:

1. Ignoring the outcome of truncate().

2. Correcting the argument of truncate().

3. Using sysseek(fd, 0, SEEK_END) to determine the random-access
   storage capacity of any type of file.
   (Perl's sysseek() promises to call POSIX lseek().
    More portability can hardly be imagined.
    In my tests any unsuitable type of Linux file yielded size 0
    which is correctly considered an error by mkdiskimage.)

4. New option -S for cylinder-wise synchronized output with
   pacifier messages. Already submitted in an earlier patch
   of Thu, 23 Jan 2014 19:01:48 +0100.

5. Without option -s :
   Zeroizing of unclaimed space, as far as known, if the size is
   given with option -M or if c is given as 0.
   (In the other cases, the unclaimed space might be much larger
    than a cylinder. For those i would propose a new option -A,
    if ever.)

I can make single-issue diffs, if desired.
But first i ask for comments:

=================================================================
--- test/mkdiskimage_orig.in	2014-01-23 12:23:34.000000000 +0100
+++ test/mkdiskimage.in	2014-01-30 17:57:30.000000000 +0100
@@ -19,6 +19,7 @@
 use bytes;
 use integer;
 use Fcntl;
+use Fcntl qw(:seek);
 use Errno;
 use Cwd;
 use IO::Handle;			# For flush()
@@ -94,6 +95,7 @@ while (defined($a = shift(@ARGV))) {
 
 ($file,$c,$h,$s) = @args;
 $c += 0;  $h += 0;  $s += 0;
+$zero_tail = 0;
 
 $pentry = 1;
 $pentry = 2 if ( $opt{'2'} );
@@ -107,7 +109,9 @@ if ( $opt{'z'} ) {
 
 if ( $opt{'M'} && $h && $s ) {
     # Specify size in megabytes, not in cylinders
+    $mb = $c;
     $c = ($c*1024*2)/($h*$s);
+    $zero_tail = ($mb * 1024 * 2 - $c * $h * $s);
 }
 
 $is_open = 0;
@@ -116,15 +120,8 @@ if ( $c == 0 && $file ne '' ) {
     $len = 0;
     if ( sysopen(OUTPUT, $file, O_RDWR, 0666) ) {
 	$is_open = 1;
-
-	if ( (@filestat = stat(OUTPUT)) && S_ISREG($filestat[2]) ) {
-	    $len = $filestat[7] >> 9;
-	} elsif ( $is_linux && S_ISBLK($filestat[2]) ) {
-	    $blksize = pack("L!", 0);
-	    if ( ioctl(OUTPUT, $BLKGETSIZE, $blksize) == 0 ) {
-		$len = unpack("L!", $blksize); # In 512-byte sectors!
-	    }
-	}
+	$len = sysseek(OUTPUT, 0, SEEK_END) >> 9;
+	sysseek(OUTPUT, 0, SEEK_SET);
     }
 
     if ( !$len ) {
@@ -133,6 +130,7 @@ if ( $c == 0 && $file ne '' ) {
     }
 
     $c = $len/($h*$s);
+    $zero_tail = ($len - $c * $h * $s);
 }
 
 if ( $file eq '' || $c < 1 || $h < 1 || $h > 256 || $s < 1 || $s > 63 ) {
@@ -145,6 +143,7 @@ if ( $file eq '' || $c < 1 || $h < 1 || 
     print STDERR "    -4    use partition entry 4 (standard for zipdisks)\n";
     print STDERR "    -i    specify the MBR ID\n";
     print STDERR "    -s    output a sparse file (don't allocate all blocks)\n";
+    print STDERR "    -S    sync after each zeroized cylinder and show count\n";
     exit 1;
 }
 
@@ -217,16 +216,41 @@ print OUTPUT "\x55\xaa";
 $totalsize = $c*$h*$s;
 $tracks    = $c*$h;
 
-# If -s is given, try to simply use truncate...
-unless ($opt{'s'} && truncate(OUTPUT, $totalsize)) {
+# If -s is given, simply use truncate. Do not care for success. Failure is
+# normal with Linux block devices.
+if ($opt{'s'}) {
+    truncate(OUTPUT, ($totalsize + $zero_tail) * 512);
+} else {
     $track = "\0" x (512*$s);
 
+    $show_pacifier = $opt{'S'};
+
     # Print fractional track
     print OUTPUT "\0" x (512 * ($s-1));
 
     for ( $i = 1 ; $i < $tracks ; $i++ ) {
+	if ( $show_pacifier && ( $i - 1 ) % $h == 0 ) {
+	    $cyl_to_write = ( $i - 1 ) / $h + 1;
+	    IO::Handle::sync(OUTPUT);
+	    print STDERR " zeroizing cylinder $cyl_to_write of $c\r";
+	}
 	print OUTPUT $track;
     }
+    if ( $zero_tail > 0 ) {
+	if ( $show_pacifier ) {
+	    IO::Handle::sync(OUTPUT);
+	    print STDERR " zeroizing $zero_tail unclaimed blocks\r";
+	}
+	$block = "\0" x 512;
+	for ( $i = 0 ; $i < $zero_tail ; $i++ ) {
+	    print OUTPUT $block;
+	}
+    } 
+    if ( $show_pacifier ) {
+	IO::Handle::sync(OUTPUT);
+	$zeroized = $totalsize + $zero_tail - 1;
+	print STDERR " $zeroized blocks zeroized.              \n";
+    }
 }
 
 # Print mtools temp file
=================================================================


Have a nice day :)

Thomas