Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 23 Jan 2010 07:55:54 +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: r202872 - head/lib/libarchive
Message-ID:  <201001230755.o0N7tsx2041441@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kientzle
Date: Sat Jan 23 07:55:53 2010
New Revision: 202872
URL: http://svn.freebsd.org/changeset/base/202872

Log:
  Use a simpler memory-management strategy for the file objects.
  Instead of trying to reference-count them and free them as soon
  as they are no longer needed, we now just keep them around and free
  them all when we release the archive object.  This fixes a number
  of minor memory leaks, especially when reading damaged archives.

Modified:
  head/lib/libarchive/archive_read_support_format_iso9660.c

Modified: head/lib/libarchive/archive_read_support_format_iso9660.c
==============================================================================
--- head/lib/libarchive/archive_read_support_format_iso9660.c	Sat Jan 23 07:54:15 2010	(r202871)
+++ head/lib/libarchive/archive_read_support_format_iso9660.c	Sat Jan 23 07:55:53 2010	(r202872)
@@ -258,9 +258,9 @@ struct content {
 
 /* In-memory storage for a directory record. */
 struct file_info {
+	struct file_info	*use_next;
 	struct file_info	*parent;
 	struct file_info	*next;
-	int		 refcount;
 	int		 subdirs;
 	uint64_t	 key;		/* Heap Key.			*/
 	uint64_t	 offset;	/* Offset on disk.		*/
@@ -331,6 +331,7 @@ struct iso9660 {
 	int64_t		previous_number;
 	struct archive_string previous_pathname;
 
+	struct file_info		*use_files;
 	struct heap_queue		 pending_files;
 	struct {
 		struct file_info	*first;
@@ -396,7 +397,8 @@ static void	parse_rockridge_TF1(struct f
 		    const unsigned char *, int);
 static void	parse_rockridge_ZF1(struct file_info *,
 		    const unsigned char *, int);
-static void	release_file(struct iso9660 *, struct file_info *);
+static void	register_file(struct iso9660 *, struct file_info *);
+static void	release_files(struct iso9660 *);
 static unsigned	toi(const void *p, int n);
 static inline void cache_add_entry(struct iso9660 *iso9660,
 		    struct file_info *file);
@@ -980,7 +982,6 @@ read_children(struct archive_read *a, st
 					}
 					con = malloc(sizeof(struct content));
 					if (con == NULL) {
-						release_file(iso9660, child);
 						archive_set_error(
 						    &a->archive, ENOMEM,
 						    "No memory for "
@@ -998,7 +999,6 @@ read_children(struct archive_read *a, st
 						multi->size += child->size;
 						if (!child->multi_extent)
 							multi = NULL;
-						release_file(iso9660, child);
 					}
 				} else
 					add_entry(iso9660, child);
@@ -1029,10 +1029,8 @@ relocate_dir(struct iso9660 *iso9660, st
 		/* This case is wrong pattern. */
 		return (0);
 	if (re->offset == file->cl_offset) {
-		re->parent->refcount--;
 		re->parent->subdirs--;
 		re->parent = file->parent;
-		re->parent->refcount++;
 		re->parent->subdirs++;
 		cache_add_to_next_of_parent(iso9660, re);
 		return (1);
@@ -1077,10 +1075,8 @@ read_entries(struct archive_read *a)
 		/*
 		 * Relocate directory which rr_moved has.
 		 */
-		while ((file = heap_get_entry(&(iso9660->cl_files))) != NULL) {
+		while ((file = heap_get_entry(&(iso9660->cl_files))) != NULL)
 			relocate_dir(iso9660, file);
-			release_file(iso9660, file);
-		}
 
 		/* If rr_moved directory still has children,
 		 * Add rr_moved into pending_files to show
@@ -1092,10 +1088,8 @@ read_entries(struct archive_read *a)
 			 * is broken), the entries won't be exposed. */
 			while ((file = heap_get_entry(&(iso9660->re_dirs))) != NULL)
 				cache_add_entry(iso9660, file);
-		} else {
+		} else
 			iso9660->rr_moved->parent->subdirs--;
-			release_file(iso9660, iso9660->rr_moved);
-		}
 	} else {
 		/*
 		 * In case ISO image is broken. If the name of rr_moved
@@ -1174,7 +1168,6 @@ archive_read_format_iso9660_read_header(
 		if (vd == &(iso9660->primary) && !iso9660->seenRockridge
 		    && iso9660->seenJoliet) {
 			/* Switch reading data from primary to joliet. */ 
-			release_file(iso9660, file);
 			vd = &(iso9660->joliet);
 			skipsize = LOGICAL_BLOCK_SIZE * vd->location;
 			skipsize -= iso9660->current_position;
@@ -1214,10 +1207,8 @@ archive_read_format_iso9660_read_header(
 
 	/* Get the next entry that appears after the current offset. */
 	r = next_entry_seek(a, iso9660, &file);
-	if (r != ARCHIVE_OK) {
-		release_file(iso9660, file);
+	if (r != ARCHIVE_OK)
 		return (r);
-	}
 
 	iso9660->entry_bytes_remaining = file->size;
 	iso9660->entry_sparse_offset = 0; /* Offset for sparse-file-aware clients. */
@@ -1227,7 +1218,6 @@ archive_read_format_iso9660_read_header(
 		    "File is beyond end-of-media: %s", file->name);
 		iso9660->entry_bytes_remaining = 0;
 		iso9660->entry_sparse_offset = 0;
-		release_file(iso9660, file);
 		return (ARCHIVE_WARN);
 	}
 
@@ -1263,7 +1253,6 @@ archive_read_format_iso9660_read_header(
 		archive_entry_unset_size(entry);
 		iso9660->entry_bytes_remaining = 0;
 		iso9660->entry_sparse_offset = 0;
-		release_file(iso9660, file);
 		return (ARCHIVE_OK);
 	}
 
@@ -1291,7 +1280,6 @@ archive_read_format_iso9660_read_header(
 		    file->offset, iso9660->current_position);
 		iso9660->entry_bytes_remaining = 0;
 		iso9660->entry_sparse_offset = 0;
-		release_file(iso9660, file);
 		return (ARCHIVE_WARN);
 	}
 
@@ -1331,8 +1319,6 @@ archive_read_format_iso9660_read_header(
 		file->exposed = 1;
 	}
 
-	release_file(iso9660, file);
-
 	if (rd_r != ARCHIVE_OK)
 		return (rd_r);
 	return (ARCHIVE_OK);
@@ -1648,14 +1634,10 @@ static int
 archive_read_format_iso9660_cleanup(struct archive_read *a)
 {
 	struct iso9660 *iso9660;
-	struct file_info *file;
 	int r = ARCHIVE_OK;
 
 	iso9660 = (struct iso9660 *)(a->format->data);
-	while ((file = cache_get_entry(iso9660)) != NULL)
-		release_file(iso9660, file);
-	while ((file = next_entry(iso9660)) != NULL)
-		release_file(iso9660, file);
+	release_files(iso9660);
 	free(iso9660->read_ce_req.reqs);
 	archive_string_free(&iso9660->pathname);
 	archive_string_free(&iso9660->previous_pathname);
@@ -1736,8 +1718,6 @@ parse_file_info(struct archive_read *a, 
 	}
 	memset(file, 0, sizeof(*file));
 	file->parent = parent;
-	if (parent != NULL)
-		parent->refcount++;
 	file->offset = iso9660->logical_block_size * (uint64_t)location;
 	file->size = toi(isodirrec + DR_size_offset, DR_size_size);
 	file->mtime = isodate7(isodirrec + DR_date_offset);
@@ -1869,8 +1849,6 @@ parse_file_info(struct archive_read *a, 
 			rr_start += iso9660->suspOffset;
 			r = parse_rockridge(a, file, rr_start, rr_end);
 			if (r != ARCHIVE_OK) {
-				if (parent != NULL)
-					parent->refcount--;
 				free(file);
 				return (NULL);
 			}
@@ -1880,6 +1858,7 @@ parse_file_info(struct archive_read *a, 
 			iso9660->opt_support_rockridge = 0;
 	}
 
+	file->nlinks = 1;/* Reset nlink. we'll calculate it later. */
 	/* Tell file's parent how many children that parent has. */
 	if (parent != NULL && (flags & 0x02) && file->cl_offset == 0)
 		parent->subdirs++;
@@ -1908,6 +1887,7 @@ parse_file_info(struct archive_read *a, 
 		fprintf(stderr, "\n");
 	}
 #endif
+	register_file(iso9660, file);
 	return (file);
 }
 
@@ -2460,18 +2440,24 @@ parse_rockridge_ZF1(struct file_info *fi
 	}
 }
 
+static void
+register_file(struct iso9660 *iso9660, struct file_info *file)
+{
+
+	file->use_next = iso9660->use_files;
+	iso9660->use_files = file;
+}
 
 static void
-release_file(struct iso9660 *iso9660, struct file_info *file)
+release_files(struct iso9660 *iso9660)
 {
-	struct file_info *parent;
 	struct content *con, *connext;
+	struct file_info *file;
 
-	if (file == NULL)
-		return;
+	file = iso9660->use_files;
+	while (file != NULL) {
+		struct file_info *next = file->use_next;
 
-	if (file->refcount == 0) {
-		parent = file->parent;
 		archive_string_free(&file->name);
 		archive_string_free(&file->symlink);
 		con = file->contents.first;
@@ -2481,10 +2467,7 @@ release_file(struct iso9660 *iso9660, st
 			con = connext;
 		}
 		free(file);
-		if (parent != NULL) {
-			parent->refcount--;
-			release_file(iso9660, parent);
-		}
+		file = next;
 	}
 }
 
@@ -2568,7 +2551,6 @@ next_cache_entry(struct iso9660 *iso9660
 			 * happen.
 			 */
 			file->next = NULL;
-			file->refcount++;
 			*empty_files.last = file;
 			empty_files.last = &(file->next);
 		} else {
@@ -2611,7 +2593,6 @@ static inline void
 cache_add_entry(struct iso9660 *iso9660, struct file_info *file)
 {
 	file->next = NULL;
-	file->refcount++;
 	*iso9660->cache_files.last = file;
 	iso9660->cache_files.last = &(file->next);
 }
@@ -2621,7 +2602,6 @@ cache_add_to_next_of_parent(struct iso96
 {
 	file->next = file->parent->next;
 	file->parent->next = file;
-	file->refcount++;
 	if (iso9660->cache_files.last == &(file->parent->next))
 		iso9660->cache_files.last = &(file->next);
 }
@@ -2633,7 +2613,6 @@ cache_get_entry(struct iso9660 *iso9660)
 
 	if ((file = iso9660->cache_files.first) != NULL) {
 		iso9660->cache_files.first = file->next;
-		file->refcount--;
 		if (iso9660->cache_files.first == NULL)
 			iso9660->cache_files.last = &(iso9660->cache_files.first);
 	}
@@ -2669,7 +2648,6 @@ heap_add_entry(struct heap_queue *heap, 
 	}
 
 	file_key = file->key = key;
-	file->refcount++;
 
 	/*
 	 * Start with hole at end, walk it up tree to find insertion point.
@@ -2703,7 +2681,6 @@ heap_get_entry(struct heap_queue *heap)
 	 * The first file in the list is the earliest; we'll return this.
 	 */
 	r = heap->files[0];
-	r->refcount--;
 
 	/*
 	 * Move the last item in the heap to the root of the tree



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