Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/usr.bin/vndcompress Fix vndcompress restart failure fallback...



details:   https://anonhg.NetBSD.org/src/rev/f6ee0830e1d1
branches:  trunk
changeset: 333835:f6ee0830e1d1
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Tue Nov 18 03:48:17 2014 +0000

description:
Fix vndcompress restart failure fallback when input is a pipe.

Defer seeking the *input* image, or winding it forward, until we are
certain we all ready in the cloop2 output, because when the input
image is a pipe, we don't get a chance to seek back to the beginning
and start from the top instead of restarting.

If restart does fail, don't try to seek the input image back to the
beginning unless we had already tried to seek or wind it forward.

Add some automatic tests for this and related cases.

XXX pullup to netbsd-7, netbsd-6

diffstat:

 usr.bin/vndcompress/Makefile      |  62 +++++++++++++++++++++++++++++++++++++-
 usr.bin/vndcompress/vndcompress.c |  42 ++++++++++++++++----------
 2 files changed, 85 insertions(+), 19 deletions(-)

diffs (175 lines):

diff -r c3d4b0c4f4e5 -r f6ee0830e1d1 usr.bin/vndcompress/Makefile
--- a/usr.bin/vndcompress/Makefile      Tue Nov 18 02:01:08 2014 +0000
+++ b/usr.bin/vndcompress/Makefile      Tue Nov 18 03:48:17 2014 +0000
@@ -77,6 +77,7 @@
        head -c 5120 < /usr/share/dict/words > ${.TARGET}.tmp \
        && mv -f ${.TARGET}.tmp ${.TARGET}
 
+# Make sure we can restart from a pipe.
 CHECKS+=       check-pipe-restart
 CLEANFILES+=   piperestart.in piperestart.in.tmp
 CLEANFILES+=   piperestart.cl2 piperestart.cl2.tmp
@@ -87,7 +88,7 @@
 piperestart.cl2restart: piperestart.cl2part vndcompress
        cp piperestart.cl2part ${.TARGET}.tmp \
        && head -c 700000 < /usr/share/dict/words \
-       | ./vndcompress -l 655360 -k 1 -rR /dev/stdin ${.TARGET}.tmp \
+       | ./vndcompress -l 655360 -k 1 -r -R /dev/stdin ${.TARGET}.tmp \
        && mv -f ${.TARGET}.tmp ${.TARGET}
 # The following rule uses ; and not && on purpose: vndcompress is
 # supposed to fail (and it is even OK to interrupt!) so we can restart
@@ -100,13 +101,53 @@
        head -c 655360 < /usr/share/dict/words > ${.TARGET}.tmp \
        && mv -f ${.TARGET}.tmp ${.TARGET}
 
+# Make sure we can restart from a pipe even if the original start was
+# corrupted, as long as we don't pass -R.
+CHECKS+=       check-pipe-badstart
+CLEANFILES+=   pipebadstart.in pipebadstart.in.tmp
+CLEANFILES+=   pipebadstart.cl2 pipebadstart.cl2.tmp
+CLEANFILES+=   pipebadstart.cl2restart pipebadstart.cl2restart.tmp
+CLEANFILES+=   pipebadstart.cl2part pipebadstart.cl2part.tmp
+check-pipe-badstart: .PHONY pipebadstart.cl2 pipebadstart.cl2restart
+       cmp ${.ALLSRC}
+pipebadstart.cl2restart: pipebadstart.cl2part vndcompress
+       cp pipebadstart.cl2part ${.TARGET}.tmp \
+       && head -c 700000 < /usr/share/dict/words \
+       | ./vndcompress -l 655360 -k 1 -r /dev/stdin ${.TARGET}.tmp \
+       && mv -f ${.TARGET}.tmp ${.TARGET}
+pipebadstart.cl2part:
+       touch ${.TARGET}
+pipebadstart.in:
+       head -c 655360 < /usr/share/dict/words > ${.TARGET}.tmp \
+       && mv -f ${.TARGET}.tmp ${.TARGET}
+
+# Make sure we can `restart' even if there's nothing there.
+CHECKS+=       check-pipe-falsestart
+CLEANFILES+=   pipefalsestart.in pipefalsestart.in.tmp
+CLEANFILES+=   pipefalsestart.cl2 pipefalsestart.cl2.tmp
+CLEANFILES+=   pipefalsestart.cl2restart pipefalsestart.cl2restart.tmp
+check-pipe-falsestart: .PHONY pipefalsestart.cl2 pipefalsestart.cl2restart
+       cmp ${.ALLSRC}
+pipefalsestart.cl2restart: vndcompress
+       rm -f ${.TARGET}.tmp \
+       && head -c 700000 < /usr/share/dict/words \
+       | ./vndcompress -l 655360 -k 1 -r /dev/stdin ${.TARGET}.tmp \
+       && mv -f ${.TARGET}.tmp ${.TARGET}
+pipefalsestart.in:
+       head -c 655360 < /usr/share/dict/words > ${.TARGET}.tmp \
+       && mv -f ${.TARGET}.tmp ${.TARGET}
+
+# Make sure we can restart from a file, simulated with `-p'.
 CHECKS+=       check-part
-CLEANFILES+=   part.orig part.cl2part part.cl2 part.out
+CLEANFILES+=   part.orig part.orig.tmp
+CLEANFILES+=   part.cl2part part.cl2part.tmp
+CLEANFILES+=   part.cl2 part.cl2.tmp
+CLEANFILES+=   part.out part.out.tmp
 check-part: .PHONY part.orig part.out
        cmp part.orig part.out
 part.cl2: part.orig part.cl2part vndcompress
        cp part.cl2part ${.TARGET}.tmp \
-       && ./vndcompress -b 512 -rR part.orig ${.TARGET}.tmp \
+       && ./vndcompress -b 512 -r -R part.orig ${.TARGET}.tmp \
        && mv -f ${.TARGET}.tmp ${.TARGET}
 part.cl2part: part.orig vndcompress
        ./vndcompress -b 512 -p 10 part.orig ${.TARGET}.tmp \
@@ -115,6 +156,21 @@
        head -c 12345 < /usr/share/dict/words > ${.TARGET}.tmp \
        && mv -f ${.TARGET}.tmp ${.TARGET}
 
+# Make sure we can `restart' even if there's nothing there.
+CHECKS+=       check-falsestart
+CLEANFILES+=   falsestart.in falsestart.in.tmp
+CLEANFILES+=   falsestart.cl2 falsestart.cl2.tmp
+CLEANFILES+=   falsestart.cl2restart falsestart.cl2restart.tmp
+check-falsestart: .PHONY falsestart.cl2 falsestart.cl2restart
+       cmp ${.ALLSRC}
+falsestart.cl2restart: vndcompress falsestart.in
+       rm -f ${.TARGET}.tmp \
+       && ./vndcompress -r falsestart.in ${.TARGET}.tmp \
+       && mv -f ${.TARGET}.tmp ${.TARGET}
+falsestart.in:
+       head -c 655360 < /usr/share/dict/words > ${.TARGET}.tmp \
+       && mv -f ${.TARGET}.tmp ${.TARGET}
+
 TESTFILES+=    smallwindow
 smallwindow.in:
        head -c 655360 < /usr/share/dict/words > ${.TARGET}.tmp \
diff -r c3d4b0c4f4e5 -r f6ee0830e1d1 usr.bin/vndcompress/vndcompress.c
--- a/usr.bin/vndcompress/vndcompress.c Tue Nov 18 02:01:08 2014 +0000
+++ b/usr.bin/vndcompress/vndcompress.c Tue Nov 18 03:48:17 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vndcompress.c,v 1.24 2014/01/25 15:31:06 riastradh Exp $       */
+/*     $NetBSD: vndcompress.c,v 1.25 2014/11/18 03:48:17 riastradh Exp $       */
 
 /*-
  * Copyright (c) 2013 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__RCSID("$NetBSD: vndcompress.c,v 1.24 2014/01/25 15:31:06 riastradh Exp $");
+__RCSID("$NetBSD: vndcompress.c,v 1.25 2014/11/18 03:48:17 riastradh Exp $");
 
 #include <sys/endian.h>
 
@@ -513,8 +513,13 @@
                                err(1, "truncate failed");
                        if (lseek(S->cloop2_fd, 0, SEEK_SET) == -1)
                                err(1, "lseek to cloop2 beginning failed");
-                       if (lseek(S->image_fd, 0, SEEK_SET) == -1)
-                               err(1, "lseek to image beginning failed");
+
+                       /* If we seeked in the input, rewind.  */
+                       if (S->blkno != 0) {
+                               if (lseek(S->image_fd, 0, SEEK_SET) == -1)
+                                       err(1,
+                                           "lseek to image beginning failed");
+                       }
                }
        }
 
@@ -672,7 +677,23 @@
        assert(1 <= blkno);
        blkno -= 1;
 
-       /* Seek to the input position.  */
+       /* Seek to the output position.  */
+       assert(last_offset <= OFF_MAX);
+       if (lseek(S->cloop2_fd, last_offset, SEEK_SET) == -1) {
+               warn("lseek output cloop2 to %"PRIx64" failed", last_offset);
+               return false;
+       }
+
+       /* Switch from reading to writing the offset table.  */
+       if (!offtab_transmogrify_read_to_write(&S->offtab, blkno))
+               return false;
+
+       /*
+        * Seek to the input position last, after all other possible
+        * failures, because if the input is a pipe, we can't change
+        * our mind, rewind, and start at the beginning instead of
+        * restarting.
+        */
        assert(S->size <= OFF_MAX);
        assert(blkno <= (S->size / S->blocksize));
        const off_t restart_position = ((off_t)blkno * (off_t)S->blocksize);
@@ -710,17 +731,6 @@
                free(buffer);
        }
 
-       /* Seek to the output position.  */
-       assert(last_offset <= OFF_MAX);
-       if (lseek(S->cloop2_fd, last_offset, SEEK_SET) == -1) {
-               warn("lseek output cloop2 to %"PRIx64" failed", last_offset);
-               return false;
-       }
-
-       /* Switch from reading to writing the offset table.  */
-       if (!offtab_transmogrify_read_to_write(&S->offtab, blkno))
-               return false;
-
        /* Start where we left off.  */
        S->blkno = blkno;
        S->offset = last_offset;



Home | Main Index | Thread Index | Old Index