Minor cleanup of the standard read/write I/O modules:

 * Use public API, don't access struct archive directly.  (People should be able to copy these into their applications as a template for custom I/O callbacks.)
 * Set "skip" only for regular files.  ("skip" allows the low-level library to catch attempts to add an archive to itself or extract over itself.)
 * Simplify the write_open functions by just calling stat() at the beginning.  Somehow, these functions had acquired some complex logic that tried to avoid the stat() call but never succeeded.

MFC after: 10 days

6 files changed, 83 insertions, 116 deletions

diff --git a/lib/libarchive/archive_read_open_fd.c b/lib/libarchive/archive_read_open_fd.c
index ad450a8..b873d07 100644
--- a/lib/libarchive/archive_read_open_fd.c
+++ b/lib/libarchive/archive_read_open_fd.c
@@ -89,8 +89,8 @@ file_open(struct archive *a, void *client_data)
 		return (ARCHIVE_FATAL);
 	}
 
-	a->skip_file_dev = st.st_dev;
-	a->skip_file_ino = st.st_ino;
+	if (S_ISREG(st.st_mode))
+		archive_read_extract_set_skip_file(a, st.st_dev, st.st_ino);
 	return (ARCHIVE_OK);
 }
 
diff --git a/lib/libarchive/archive_read_open_file.c b/lib/libarchive/archive_read_open_file.c
index 0da7d7c..1538c5b 100644
--- a/lib/libarchive/archive_read_open_file.c
+++ b/lib/libarchive/archive_read_open_file.c
@@ -110,9 +110,10 @@ file_open(struct archive *a, void *client_data)
 		return (ARCHIVE_FATAL);
 	}
 	if (fstat(mine->fd, &st) == 0) {
-		/* Set dev/ino of archive file so extract won't overwrite. */
-		a->skip_file_dev = st.st_dev;
-		a->skip_file_ino = st.st_ino;
+		/* If we're reading a file from disk, ensure that we don't
+		   overwrite it with an extracted file. */
+		if (S_ISREG(st.st_mode))
+			archive_read_extract_set_skip_file(a, st.st_dev, st.st_ino);
 		/* Remember mode so close can decide whether to flush. */
 		mine->st_mode = st.st_mode;
 	} else {
diff --git a/lib/libarchive/archive_read_open_filename.c b/lib/libarchive/archive_read_open_filename.c
index 0da7d7c..1538c5b 100644
--- a/lib/libarchive/archive_read_open_filename.c
+++ b/lib/libarchive/archive_read_open_filename.c
@@ -110,9 +110,10 @@ file_open(struct archive *a, void *client_data)
 		return (ARCHIVE_FATAL);
 	}
 	if (fstat(mine->fd, &st) == 0) {
-		/* Set dev/ino of archive file so extract won't overwrite. */
-		a->skip_file_dev = st.st_dev;
-		a->skip_file_ino = st.st_ino;
+		/* If we're reading a file from disk, ensure that we don't
+		   overwrite it with an extracted file. */
+		if (S_ISREG(st.st_mode))
+			archive_read_extract_set_skip_file(a, st.st_dev, st.st_ino);
 		/* Remember mode so close can decide whether to flush. */
 		mine->st_mode = st.st_mode;
 	} else {
diff --git a/lib/libarchive/archive_write_open_fd.c b/lib/libarchive/archive_write_open_fd.c
index b61ee08..87acf88 100644
--- a/lib/libarchive/archive_write_open_fd.c
+++ b/lib/libarchive/archive_write_open_fd.c
@@ -74,44 +74,35 @@ static int
 file_open(struct archive *a, void *client_data)
 {
 	struct write_fd_data *mine;
-	struct stat st, *pst;
+	struct stat st;
 
-	pst = NULL;
 	mine = (struct write_fd_data *)client_data;
 
+	if (fstat(mine->fd, &st) != 0) {
+		archive_set_error(a, errno, "Couldn't stat fd %d", mine->fd);
+		return (ARCHIVE_FATAL);
+	}
+
 	/*
-	 * If client hasn't explicitly set the last block handling,
-	 * then set it here: If the output is a block or character
-	 * device, pad the last block, otherwise leave it unpadded.
+	 * If this is a regular file, don't add it to itself.
 	 */
-	if (mine->fd >= 0 && a->bytes_in_last_block < 0) {
-		/* Last block will be fully padded. */
-		if (fstat(mine->fd, &st) == 0) {
-			pst = &st;
-			if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode) ||
-			    S_ISFIFO(st.st_mode))
-				archive_write_set_bytes_in_last_block(a, 0);
-			else
-				archive_write_set_bytes_in_last_block(a, 1);
-		}
-	}
+	if (S_ISREG(st.st_mode))
+		archive_write_set_skip_file(a, st.st_dev, st.st_ino);
 
-	if (mine->fd == 1) {
-		if (a->bytes_in_last_block < 0) /* Still default? */
+	/*
+	 * If client hasn't explicitly set the last block handling,
+	 * then set it here.
+	 */
+	if (archive_write_get_bytes_in_last_block(a) < 0) {
+		/* If the output is a block or character device, fifo,
+		 * or stdout, pad the last block, otherwise leave it
+		 * unpadded. */
+		if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode) ||
+		    S_ISFIFO(st.st_mode) || (mine->fd == 1))
 			/* Last block will be fully padded. */
 			archive_write_set_bytes_in_last_block(a, 0);
-	}
-
-	if (mine->fd < 0) {
-		archive_set_error(a, errno, "Failed to open");
-		return (ARCHIVE_FATAL);
-	}
-
-	if (pst == NULL && fstat(mine->fd, &st) == 0)
-		pst = &st;
-	if (pst == NULL) {
-		archive_set_error(a, errno, "Couldn't stat fd %d", mine->fd);
-		return (ARCHIVE_FATAL);
+		else
+			archive_write_set_bytes_in_last_block(a, 1);
 	}
 
 	return (ARCHIVE_OK);
diff --git a/lib/libarchive/archive_write_open_file.c b/lib/libarchive/archive_write_open_file.c
index 8cf5995..a464a01 100644
--- a/lib/libarchive/archive_write_open_file.c
+++ b/lib/libarchive/archive_write_open_file.c
@@ -88,53 +88,42 @@ file_open(struct archive *a, void *client_data)
 {
 	int flags;
 	struct write_file_data *mine;
-	struct stat st, *pst;
+	struct stat st;
 
-	pst = NULL;
 	mine = (struct write_file_data *)client_data;
 	flags = O_WRONLY | O_CREAT | O_TRUNC;
 
+	/*
+	 * Open the file.
+	 */
 	if (mine->filename[0] != '\0') {
 		mine->fd = open(mine->filename, flags, 0666);
-
-		/*
-		 * If client hasn't explicitly set the last block
-		 * handling, then set it here: If the output is a
-		 * block or character device, pad the last block,
-		 * otherwise leave it unpadded.
-		 */
-		if (mine->fd >= 0 && a->bytes_in_last_block < 0) {
-			if (fstat(mine->fd, &st) == 0) {
-				pst = &st;
-				if (S_ISCHR(st.st_mode) ||
-				    S_ISBLK(st.st_mode) ||
-				    S_ISFIFO(st.st_mode))
-					/* Pad last block. */
-					archive_write_set_bytes_in_last_block(a, 0);
-				else
-					/* Don't pad last block. */
-					archive_write_set_bytes_in_last_block(a, 1);
-			}
+		if (mine->fd < 0) {
+			archive_set_error(a, errno, "Failed to open '%s'",
+			    mine->filename);
+			return (ARCHIVE_FATAL);
 		}
 	} else {
+		/*
+		 * NULL filename is stdout.
+		 */
 		mine->fd = 1;
-		if (a->bytes_in_last_block < 0) /* Still default? */
-			/* Last block will be fully padded. */
+		/* By default, pad archive when writing to stdout. */
+		if (archive_write_get_bytes_in_last_block(a) < 0)
 			archive_write_set_bytes_in_last_block(a, 0);
 	}
 
-	if (mine->fd < 0) {
-		archive_set_error(a, errno, "Failed to open '%s'",
-		    mine->filename);
-		return (ARCHIVE_FATAL);
-	}
-
-	if (pst == NULL && fstat(mine->fd, &st) == 0)
-		pst = &st;
-	if (pst == NULL) {
-		archive_set_error(a, errno, "Couldn't stat '%s'",
-		    mine->filename);
-		return (ARCHIVE_FATAL);
+	/*
+	 * Set up default last block handling.
+	 */
+	if (archive_write_get_bytes_in_last_block(a) < 0) {
+		if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode) ||
+		    S_ISFIFO(st.st_mode))
+			/* Pad last block when writing to device or FIFO. */
+			archive_write_set_bytes_in_last_block(a, 0);
+		else
+			/* Don't pad last block otherwise. */
+			archive_write_set_bytes_in_last_block(a, 1);
 	}
 
 	/*
@@ -142,10 +131,8 @@ file_open(struct archive *a, void *client_data)
 	 * itself.  If it's a device file, it's okay to add the device
 	 * entry to the output archive.
 	 */
-	if (S_ISREG(pst->st_mode)) {
-		a->skip_file_dev = pst->st_dev;
-		a->skip_file_ino = pst->st_ino;
-	}
+	if (S_ISREG(st.st_mode))
+		archive_write_set_skip_file(a, st.st_dev, st.st_ino);
 
 	return (ARCHIVE_OK);
 }
diff --git a/lib/libarchive/archive_write_open_filename.c b/lib/libarchive/archive_write_open_filename.c
index 8cf5995..a464a01 100644
--- a/lib/libarchive/archive_write_open_filename.c
+++ b/lib/libarchive/archive_write_open_filename.c
@@ -88,53 +88,42 @@ file_open(struct archive *a, void *client_data)
 {
 	int flags;
 	struct write_file_data *mine;
-	struct stat st, *pst;
+	struct stat st;
 
-	pst = NULL;
 	mine = (struct write_file_data *)client_data;
 	flags = O_WRONLY | O_CREAT | O_TRUNC;
 
+	/*
+	 * Open the file.
+	 */
 	if (mine->filename[0] != '\0') {
 		mine->fd = open(mine->filename, flags, 0666);
-
-		/*
-		 * If client hasn't explicitly set the last block
-		 * handling, then set it here: If the output is a
-		 * block or character device, pad the last block,
-		 * otherwise leave it unpadded.
-		 */
-		if (mine->fd >= 0 && a->bytes_in_last_block < 0) {
-			if (fstat(mine->fd, &st) == 0) {
-				pst = &st;
-				if (S_ISCHR(st.st_mode) ||
-				    S_ISBLK(st.st_mode) ||
-				    S_ISFIFO(st.st_mode))
-					/* Pad last block. */
-					archive_write_set_bytes_in_last_block(a, 0);
-				else
-					/* Don't pad last block. */
-					archive_write_set_bytes_in_last_block(a, 1);
-			}
+		if (mine->fd < 0) {
+			archive_set_error(a, errno, "Failed to open '%s'",
+			    mine->filename);
+			return (ARCHIVE_FATAL);
 		}
 	} else {
+		/*
+		 * NULL filename is stdout.
+		 */
 		mine->fd = 1;
-		if (a->bytes_in_last_block < 0) /* Still default? */
-			/* Last block will be fully padded. */
+		/* By default, pad archive when writing to stdout. */
+		if (archive_write_get_bytes_in_last_block(a) < 0)
 			archive_write_set_bytes_in_last_block(a, 0);
 	}
 
-	if (mine->fd < 0) {
-		archive_set_error(a, errno, "Failed to open '%s'",
-		    mine->filename);
-		return (ARCHIVE_FATAL);
-	}
-
-	if (pst == NULL && fstat(mine->fd, &st) == 0)
-		pst = &st;
-	if (pst == NULL) {
-		archive_set_error(a, errno, "Couldn't stat '%s'",
-		    mine->filename);
-		return (ARCHIVE_FATAL);
+	/*
+	 * Set up default last block handling.
+	 */
+	if (archive_write_get_bytes_in_last_block(a) < 0) {
+		if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode) ||
+		    S_ISFIFO(st.st_mode))
+			/* Pad last block when writing to device or FIFO. */
+			archive_write_set_bytes_in_last_block(a, 0);
+		else
+			/* Don't pad last block otherwise. */
+			archive_write_set_bytes_in_last_block(a, 1);
 	}
 
 	/*
@@ -142,10 +131,8 @@ file_open(struct archive *a, void *client_data)
 	 * itself.  If it's a device file, it's okay to add the device
 	 * entry to the output archive.
 	 */
-	if (S_ISREG(pst->st_mode)) {
-		a->skip_file_dev = pst->st_dev;
-		a->skip_file_ino = pst->st_ino;
-	}
+	if (S_ISREG(st.st_mode))
+		archive_write_set_skip_file(a, st.st_dev, st.st_ino);
 
 	return (ARCHIVE_OK);
 }