Module Name:    src
Committed By:   snj
Date:           Wed Mar 18 08:21:47 UTC 2015

Modified Files:
        src/usr.bin/vndcompress [netbsd-7]: Makefile vndcompress.c

Log Message:
Pull up following revision(s) (requested by riastradh in ticket #609):
        usr.bin/vndcompress/Makefile: revision 1.14
        usr.bin/vndcompress/vndcompress.c: revision 1.25
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


To generate a diff of this commit:
cvs rdiff -u -r1.13 -r1.13.4.1 src/usr.bin/vndcompress/Makefile
cvs rdiff -u -r1.24 -r1.24.4.1 src/usr.bin/vndcompress/vndcompress.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/usr.bin/vndcompress/Makefile
diff -u src/usr.bin/vndcompress/Makefile:1.13 src/usr.bin/vndcompress/Makefile:1.13.4.1
--- src/usr.bin/vndcompress/Makefile:1.13	Wed Jan 22 06:18:00 2014
+++ src/usr.bin/vndcompress/Makefile	Wed Mar 18 08:21:47 2015
@@ -77,6 +77,7 @@ tentinyblock.in:
 	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 @@ check-pipe-restart: .PHONY piperestart.c
 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 @@ piperestart.in:
 	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 @@ part.orig:
 	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 \

Index: src/usr.bin/vndcompress/vndcompress.c
diff -u src/usr.bin/vndcompress/vndcompress.c:1.24 src/usr.bin/vndcompress/vndcompress.c:1.24.4.1
--- src/usr.bin/vndcompress/vndcompress.c:1.24	Sat Jan 25 15:31:06 2014
+++ src/usr.bin/vndcompress/vndcompress.c	Wed Mar 18 08:21:47 2015
@@ -1,4 +1,4 @@
-/*	$NetBSD: vndcompress.c,v 1.24 2014/01/25 15:31:06 riastradh Exp $	*/
+/*	$NetBSD: vndcompress.c,v 1.24.4.1 2015/03/18 08:21:47 snj 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.24.4.1 2015/03/18 08:21:47 snj Exp $");
 
 #include <sys/endian.h>
 
@@ -513,8 +513,13 @@ compress_init(int argc, char **argv, con
 				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 @@ compress_restart(struct compress_state *
 	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 @@ compress_restart(struct compress_state *
 		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;

Reply via email to