Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 17 Apr 2009 00:50:00 +0000 (UTC)
From:      Tim Kientzle <kientzle@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r191171 - head/lib/libarchive
Message-ID:  <200904170050.n3H0o0fS098969@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kientzle
Date: Fri Apr 17 00:50:00 2009
New Revision: 191171
URL: http://svn.freebsd.org/changeset/base/191171

Log:
  Don't use the open callback, which is deprecated (because it's
  never necessary).  Also, simplify just a tad by delegating
  to read_open_fd() when we know the file descriptor, instead
  of duplicating that logic.

Modified:
  head/lib/libarchive/archive_read_open_fd.c
  head/lib/libarchive/archive_read_open_file.c
  head/lib/libarchive/archive_read_open_filename.c

Modified: head/lib/libarchive/archive_read_open_fd.c
==============================================================================
--- head/lib/libarchive/archive_read_open_fd.c	Fri Apr 17 00:47:16 2009	(r191170)
+++ head/lib/libarchive/archive_read_open_fd.c	Fri Apr 17 00:50:00 2009	(r191171)
@@ -52,7 +52,6 @@ struct read_fd_data {
 };
 
 static int	file_close(struct archive *, void *);
-static int	file_open(struct archive *, void *);
 static ssize_t	file_read(struct archive *, void *, const void **buff);
 #if ARCHIVE_API_VERSION < 2
 static ssize_t	file_skip(struct archive *, void *, size_t request);
@@ -63,50 +62,41 @@ static off_t	file_skip(struct archive *,
 int
 archive_read_open_fd(struct archive *a, int fd, size_t block_size)
 {
+	struct stat st;
 	struct read_fd_data *mine;
+	void *b;
 
-	mine = (struct read_fd_data *)malloc(sizeof(*mine));
-	if (mine == NULL) {
-		archive_set_error(a, ENOMEM, "No memory");
+	if (fstat(fd, &st) != 0) {
+		archive_set_error(a, errno, "Can't stat fd %d", fd);
 		return (ARCHIVE_FATAL);
 	}
-	mine->block_size = block_size;
-	mine->buffer = malloc(mine->block_size);
-	if (mine->buffer == NULL) {
+
+	mine = (struct read_fd_data *)malloc(sizeof(*mine));
+	b = malloc(block_size);
+	if (mine == NULL || b == NULL) {
 		archive_set_error(a, ENOMEM, "No memory");
 		free(mine);
+		free(b);
 		return (ARCHIVE_FATAL);
 	}
+	mine->block_size = block_size;
+	mine->buffer = b;
 	mine->fd = fd;
-	/* lseek() hardly ever works, so disable it by default.  See below. */
-	mine->can_skip = 0;
-	return (archive_read_open2(a, mine, file_open, file_read, file_skip, file_close));
-}
-
-static int
-file_open(struct archive *a, void *client_data)
-{
-	struct read_fd_data *mine = (struct read_fd_data *)client_data;
-	struct stat st;
-
-	if (fstat(mine->fd, &st) != 0) {
-		archive_set_error(a, errno, "Can't stat fd %d", mine->fd);
-		return (ARCHIVE_FATAL);
-	}
-
+	/*
+	 * Skip support is a performance optimization for anything
+	 * that supports lseek().  On FreeBSD, only regular files and
+	 * raw disk devices support lseek() and there's no portable
+	 * way to determine if a device is a raw disk device, so we
+	 * only enable this optimization for regular files.
+	 */
 	if (S_ISREG(st.st_mode)) {
 		archive_read_extract_set_skip_file(a, st.st_dev, st.st_ino);
-		/*
-		 * Enabling skip here is a performance optimization for
-		 * anything that supports lseek().  On FreeBSD, only
-		 * regular files and raw disk devices support lseek() and
-		 * there's no portable way to determine if a device is
-		 * a raw disk device, so we only enable this optimization
-		 * for regular files.
-		 */
 		mine->can_skip = 1;
-	}
-	return (ARCHIVE_OK);
+	} else
+		mine->can_skip = 0;
+
+	return (archive_read_open2(a, mine,
+		NULL, file_read, file_skip, file_close));
 }
 
 static ssize_t
@@ -180,8 +170,7 @@ file_close(struct archive *a, void *clie
 	struct read_fd_data *mine = (struct read_fd_data *)client_data;
 
 	(void)a; /* UNUSED */
-	if (mine->buffer != NULL)
-		free(mine->buffer);
+	free(mine->buffer);
 	free(mine);
 	return (ARCHIVE_OK);
 }

Modified: head/lib/libarchive/archive_read_open_file.c
==============================================================================
--- head/lib/libarchive/archive_read_open_file.c	Fri Apr 17 00:47:16 2009	(r191170)
+++ head/lib/libarchive/archive_read_open_file.c	Fri Apr 17 00:50:00 2009	(r191171)
@@ -55,7 +55,6 @@ struct read_FILE_data {
 };
 
 static int	file_close(struct archive *, void *);
-static int	file_open(struct archive *, void *);
 static ssize_t	file_read(struct archive *, void *, const void **buff);
 #if ARCHIVE_API_VERSION < 2
 static ssize_t	file_skip(struct archive *, void *, size_t request);
@@ -66,45 +65,36 @@ static off_t	file_skip(struct archive *,
 int
 archive_read_open_FILE(struct archive *a, FILE *f)
 {
+	struct stat st;
 	struct read_FILE_data *mine;
+	size_t block_size = 128 * 1024;
+	void *b;
 
 	mine = (struct read_FILE_data *)malloc(sizeof(*mine));
-	if (mine == NULL) {
-		archive_set_error(a, ENOMEM, "No memory");
-		return (ARCHIVE_FATAL);
-	}
-	mine->block_size = 128 * 1024;
-	mine->buffer = malloc(mine->block_size);
-	if (mine->buffer == NULL) {
+	b = malloc(block_size);
+	if (mine == NULL || b == NULL) {
 		archive_set_error(a, ENOMEM, "No memory");
 		free(mine);
+		free(b);
 		return (ARCHIVE_FATAL);
 	}
+	mine->block_size = block_size;
+	mine->buffer = b;
 	mine->f = f;
-	/* Suppress skip by default. See below. */
-	mine->can_skip = 0;
-	return (archive_read_open2(a, mine, file_open, file_read,
-		    file_skip, file_close));
-}
-
-static int
-file_open(struct archive *a, void *client_data)
-{
-	struct read_FILE_data *mine = (struct read_FILE_data *)client_data;
-	struct stat st;
-
 	/*
-	 * If we can't fstat() the file, it may just be that
-	 * it's not a file.  (FILE * objects can wrap many kinds
-	 * of I/O streams.)
+	 * If we can't fstat() the file, it may just be that it's not
+	 * a file.  (FILE * objects can wrap many kinds of I/O
+	 * streams, some of which don't support fileno()).)
 	 */
 	if (fstat(fileno(mine->f), &st) == 0 && S_ISREG(st.st_mode)) {
 		archive_read_extract_set_skip_file(a, st.st_dev, st.st_ino);
-		/* Enable the seek optimization for regular files. */
+		/* Enable the seek optimization only for regular files. */
 		mine->can_skip = 1;
-	}
+	} else
+		mine->can_skip = 0;
 
-	return (ARCHIVE_OK);
+	return (archive_read_open2(a, mine, NULL, file_read,
+		    file_skip, file_close));
 }
 
 static ssize_t

Modified: head/lib/libarchive/archive_read_open_filename.c
==============================================================================
--- head/lib/libarchive/archive_read_open_filename.c	Fri Apr 17 00:47:16 2009	(r191170)
+++ head/lib/libarchive/archive_read_open_filename.c	Fri Apr 17 00:50:00 2009	(r191171)
@@ -61,7 +61,6 @@ struct read_file_data {
 };
 
 static int	file_close(struct archive *, void *);
-static int	file_open(struct archive *, void *);
 static ssize_t	file_read(struct archive *, void *, const void **buff);
 #if ARCHIVE_API_VERSION < 2
 static ssize_t	file_skip(struct archive *, void *, size_t request);
@@ -80,78 +79,54 @@ int
 archive_read_open_filename(struct archive *a, const char *filename,
     size_t block_size)
 {
+	struct stat st;
 	struct read_file_data *mine;
+	void *b;
+	int fd;
 
-	if (filename == NULL || filename[0] == '\0') {
-		mine = (struct read_file_data *)malloc(sizeof(*mine));
-		if (mine == NULL) {
-			archive_set_error(a, ENOMEM, "No memory");
-			return (ARCHIVE_FATAL);
-		}
-		mine->filename[0] = '\0';
-	} else {
-		mine = (struct read_file_data *)malloc(sizeof(*mine) + strlen(filename));
-		if (mine == NULL) {
-			archive_set_error(a, ENOMEM, "No memory");
-			return (ARCHIVE_FATAL);
-		}
-		strcpy(mine->filename, filename);
-	}
-	mine->block_size = block_size;
-	mine->buffer = NULL;
-	mine->fd = -1;
-	/* lseek() almost never works; disable it by default.  See below. */
-	mine->can_skip = 0;
-	return (archive_read_open2(a, mine, file_open, file_read, file_skip, file_close));
-}
+	if (filename == NULL || filename[0] == '\0')
+		return (archive_read_open_fd(a, 0, block_size));
 
-static int
-file_open(struct archive *a, void *client_data)
-{
-	struct read_file_data *mine = (struct read_file_data *)client_data;
-	struct stat st;
-
-	mine->buffer = malloc(mine->block_size);
-	if (mine->buffer == NULL) {
-		archive_set_error(a, ENOMEM, "No memory");
+	fd = open(filename, O_RDONLY | O_BINARY);
+	if (fd < 0) {
+		archive_set_error(a, errno, "Failed to open '%s'", filename);
 		return (ARCHIVE_FATAL);
 	}
-	if (mine->filename[0] != '\0')
-		mine->fd = open(mine->filename, O_RDONLY | O_BINARY);
-	else
-		mine->fd = 0; /* Fake "open" for stdin. */
-	if (mine->fd < 0) {
-		archive_set_error(a, errno, "Failed to open '%s'",
-		    mine->filename);
+	if (fstat(fd, &st) != 0) {
+		archive_set_error(a, errno, "Can't stat '%s'", filename);
 		return (ARCHIVE_FATAL);
 	}
-	if (fstat(mine->fd, &st) == 0) {
-		/* 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);
-			/*
-			 * Enabling skip here is a performance
-			 * optimization for anything that supports
-			 * lseek().  On FreeBSD, only regular files
-			 * and raw disk devices support lseek() and
-			 * there's no portable way to determine if a
-			 * device is a raw disk device, so we only
-			 * enable this optimization for regular files.
-			 */
-			mine->can_skip = 1;
-		}
-		/* Remember mode so close can decide whether to flush. */
-		mine->st_mode = st.st_mode;
-	} else {
-		if (mine->filename[0] == '\0')
-			archive_set_error(a, errno, "Can't stat stdin");
-		else
-			archive_set_error(a, errno, "Can't stat '%s'",
-			    mine->filename);
+
+	mine = (struct read_file_data *)malloc(sizeof(*mine) + strlen(filename));
+	b = malloc(block_size);
+	if (mine == NULL || b == NULL) {
+		archive_set_error(a, ENOMEM, "No memory");
+		free(mine);
+		free(b);
 		return (ARCHIVE_FATAL);
 	}
-	return (0);
+	strcpy(mine->filename, filename);
+	mine->block_size = block_size;
+	mine->buffer = b;
+	mine->fd = fd;
+	/* Remember mode so close can decide whether to flush. */
+	mine->st_mode = st.st_mode;
+	/* 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);
+		/*
+		 * Skip is a performance optimization for anything
+		 * that supports lseek().  Generally, that only
+		 * includes regular files and possibly raw disk
+		 * devices, but there's no good portable way to detect
+		 * raw disks.
+		 */
+		mine->can_skip = 1;
+	} else
+		mine->can_skip = 0;
+	return (archive_read_open2(a, mine,
+		NULL, file_read, file_skip, file_close));
 }
 
 static ssize_t
@@ -163,11 +138,8 @@ file_read(struct archive *a, void *clien
 	*buff = mine->buffer;
 	bytes_read = read(mine->fd, mine->buffer, mine->block_size);
 	if (bytes_read < 0) {
-		if (mine->filename[0] == '\0')
-			archive_set_error(a, errno, "Error reading stdin");
-		else
-			archive_set_error(a, errno, "Error reading '%s'",
-			    mine->filename);
+		archive_set_error(a, errno, "Error reading '%s'",
+		    mine->filename);
 	}
 	return (bytes_read);
 }
@@ -217,15 +189,8 @@ file_skip(struct archive *a, void *clien
 		 * likely caused by a programmer error (too large request)
 		 * or a corrupted archive file.
 		 */
-		if (mine->filename[0] == '\0')
-			/*
-			 * Should never get here, since lseek() on stdin ought
-			 * to return an ESPIPE error.
-			 */
-			archive_set_error(a, errno, "Error seeking in stdin");
-		else
-			archive_set_error(a, errno, "Error seeking in '%s'",
-			    mine->filename);
+		archive_set_error(a, errno, "Error seeking in '%s'",
+		    mine->filename);
 		return (-1);
 	}
 	return (new_offset - old_offset);
@@ -259,9 +224,7 @@ file_close(struct archive *a, void *clie
 				    mine->block_size);
 			} while (bytesRead > 0);
 		}
-		/* If a named file was opened, then it needs to be closed. */
-		if (mine->filename[0] != '\0')
-			close(mine->fd);
+		close(mine->fd);
 	}
 	free(mine->buffer);
 	free(mine);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200904170050.n3H0o0fS098969>