Cleanup: match xattr reader API closer to id table API

Instead of creating everything in the "create" function, cleanup and
create/initialize stuff in a "load" function. This allows the xattr
reader to be reset/re-used and adds the benefit of not having to
lug around references to the super block, compressor and file (altough
the later two are hidden inside the meta reader).

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>

4 files changed, 69 insertions, 58 deletions

diff --git a/include/sqfs/xattr_reader.h b/include/sqfs/xattr_reader.h
index 7912de4..54ffc12 100644
--- a/include/sqfs/xattr_reader.h
+++ b/include/sqfs/xattr_reader.h
@@ -83,18 +83,12 @@ extern "C" {
  * Do not destroy any of the pointed to objects before destroying the xattr
  * reader.
  *
- * @param file A pointer to a file object that contains the SquashFS filesystem
- * @param super A pointer to the SquashFS super block required to find the
- *              location tables.
- * @param cmp A pointer to a compressor used to uncompress the loaded meta data
- *            blocks.
+ * @param flags Currently must be set to 0, or creation will fail.
  *
  * @return A pointer to a new xattr reader instance on success, NULL on
  *         allocation failure.
  */
-SQFS_API sqfs_xattr_reader_t *sqfs_xattr_reader_create(sqfs_file_t *file,
-						       sqfs_super_t *super,
-						       sqfs_compressor_t *cmp);
+SQFS_API sqfs_xattr_reader_t *sqfs_xattr_reader_create(sqfs_u32 flags);
 
 /**
  * @brief Load the locations of the xattr meta data blocks into memory
@@ -104,9 +98,18 @@ SQFS_API sqfs_xattr_reader_t *sqfs_xattr_reader_create(sqfs_file_t *file,
  * This function must be called explicitly after an xattr reader has been
  * created to load the actual location table from disk.
  *
+ * @param xr A pointer to an xattr reader instance.
+ * @param super A pointer to the SquashFS super block required to find the
+ *              location tables.
+ * @param file A pointer to a file object that contains the SquashFS filesystem.
+ * @param cmp A pointer to a compressor used to uncompress the loaded meta data
+ *            blocks.
+ *
  * @return Zero on success, a negative @ref E_SQFS_ERROR value on failure.
  */
-SQFS_API int sqfs_xattr_reader_load_locations(sqfs_xattr_reader_t *xr);
+SQFS_API int sqfs_xattr_reader_load(sqfs_xattr_reader_t *xr,
+				    const sqfs_super_t *super,
+				    sqfs_file_t *file, sqfs_compressor_t *cmp);
 
 /**
  * @brief Resolve an xattr index from an inode to an xattr description
diff --git a/lib/sqfs/xattr_reader.c b/lib/sqfs/xattr_reader.c
index 1469a90..bad160d 100644
--- a/lib/sqfs/xattr_reader.c
+++ b/lib/sqfs/xattr_reader.c
@@ -24,6 +24,7 @@ struct sqfs_xattr_reader_t {
 	sqfs_object_t base;
 
 	sqfs_u64 xattr_start;
+	sqfs_u64 xattr_end;
 
 	size_t num_id_blocks;
 	size_t num_ids;
@@ -32,8 +33,6 @@ struct sqfs_xattr_reader_t {
 
 	sqfs_meta_reader_t *idrd;
 	sqfs_meta_reader_t *kvrd;
-	sqfs_super_t *super;
-	sqfs_file_t *file;
 };
 
 static sqfs_object_t *xattr_reader_copy(const sqfs_object_t *obj)
@@ -94,23 +93,40 @@ static void xattr_reader_destroy(sqfs_object_t *obj)
 	free(xr);
 }
 
-int sqfs_xattr_reader_load_locations(sqfs_xattr_reader_t *xr)
+int sqfs_xattr_reader_load(sqfs_xattr_reader_t *xr, const sqfs_super_t *super,
+			   sqfs_file_t *file, sqfs_compressor_t *cmp)
 {
 	sqfs_xattr_id_table_t idtbl;
 	size_t i;
 	int err;
 
-	if (xr->super->flags & SQFS_FLAG_NO_XATTRS)
+	/* sanity check */
+	if (super->flags & SQFS_FLAG_NO_XATTRS)
 		return 0;
 
-	if (xr->super->xattr_id_table_start == 0xFFFFFFFFFFFFFFFF)
+	if (super->xattr_id_table_start == 0xFFFFFFFFFFFFFFFF)
 		return 0;
 
-	if (xr->super->xattr_id_table_start >= xr->super->bytes_used)
+	if (super->xattr_id_table_start >= super->bytes_used)
 		return SQFS_ERROR_OUT_OF_BOUNDS;
 
-	err = xr->file->read_at(xr->file, xr->super->xattr_id_table_start,
-				&idtbl, sizeof(idtbl));
+	/* cleanup pre-existing data */
+	if (xr->idrd != NULL) {
+		sqfs_destroy(xr->idrd);
+		xr->idrd = NULL;
+	}
+
+	if (xr->kvrd != NULL) {
+		sqfs_destroy(xr->idrd);
+		xr->idrd = NULL;
+	}
+
+	free(xr->id_block_starts);
+	xr->id_block_starts = NULL;
+
+	/* read the locations table */
+	err = file->read_at(file, super->xattr_id_table_start,
+			    &idtbl, sizeof(idtbl));
 	if (err)
 		return err;
 
@@ -129,24 +145,38 @@ int sqfs_xattr_reader_load_locations(sqfs_xattr_reader_t *xr)
 		return SQFS_ERROR_ALLOC;
 	}
 
-	err = xr->file->read_at(xr->file,
-				xr->super->xattr_id_table_start + sizeof(idtbl),
-				xr->id_block_starts,
-				sizeof(sqfs_u64) * xr->num_id_blocks);
+	err = file->read_at(file, super->xattr_id_table_start + sizeof(idtbl),
+			    xr->id_block_starts,
+			    sizeof(sqfs_u64) * xr->num_id_blocks);
 	if (err)
-		goto fail;
+		goto fail_blocks;
 
 	for (i = 0; i < xr->num_id_blocks; ++i) {
 		xr->id_block_starts[i] = le64toh(xr->id_block_starts[i]);
 
-		if (xr->id_block_starts[i] > xr->super->bytes_used) {
+		if (xr->id_block_starts[i] > super->bytes_used) {
 			err = SQFS_ERROR_OUT_OF_BOUNDS;
-			goto fail;
+			goto fail_blocks;
 		}
 	}
 
+	/* create the meta data readers */
+	xr->idrd = sqfs_meta_reader_create(file, cmp, super->id_table_start,
+					   super->bytes_used);
+	if (xr->idrd == NULL)
+		goto fail_blocks;
+
+	xr->kvrd = sqfs_meta_reader_create(file, cmp, super->id_table_start,
+					   super->bytes_used);
+	if (xr->kvrd == NULL)
+		goto fail_idrd;
+
+	xr->xattr_end = super->bytes_used;
 	return 0;
-fail:
+fail_idrd:
+	sqfs_destroy(xr->idrd);
+	xr->idrd = NULL;
+fail_blocks:
 	free(xr->id_block_starts);
 	xr->id_block_starts = NULL;
 	return err;
@@ -216,7 +246,7 @@ int sqfs_xattr_reader_read_value(sqfs_xattr_reader_t *xr,
 		sqfs_meta_reader_get_position(xr->kvrd, &start, &offset);
 
 		new_start = xr->xattr_start + (ref >> 16);
-		if (new_start >= xr->super->bytes_used)
+		if (new_start >= xr->xattr_end)
 			return SQFS_ERROR_OUT_OF_BOUNDS;
 
 		new_offset = ref & 0xFFFF;
@@ -302,40 +332,18 @@ int sqfs_xattr_reader_get_desc(sqfs_xattr_reader_t *xr, sqfs_u32 idx,
 	return 0;
 }
 
-sqfs_xattr_reader_t *sqfs_xattr_reader_create(sqfs_file_t *file,
-					      sqfs_super_t *super,
-					      sqfs_compressor_t *cmp)
+sqfs_xattr_reader_t *sqfs_xattr_reader_create(sqfs_u32 flags)
 {
-	sqfs_xattr_reader_t *xr = calloc(1, sizeof(*xr));
+	sqfs_xattr_reader_t *xr;
+
+	if (flags != 0)
+		return NULL;
 
+	xr = calloc(1, sizeof(*xr));
 	if (xr == NULL)
 		return NULL;
 
 	((sqfs_object_t *)xr)->copy = xattr_reader_copy;
 	((sqfs_object_t *)xr)->destroy = xattr_reader_destroy;
-	xr->file = file;
-	xr->super = super;
-
-	if (super->flags & SQFS_FLAG_NO_XATTRS)
-		return xr;
-
-	if (super->xattr_id_table_start == 0xFFFFFFFFFFFFFFFF)
-		return xr;
-
-	xr->idrd = sqfs_meta_reader_create(file, cmp,
-					   super->id_table_start,
-					   super->bytes_used);
-	if (xr->idrd == NULL)
-		goto fail;
-
-	xr->kvrd = sqfs_meta_reader_create(file, cmp,
-					   super->id_table_start,
-					   super->bytes_used);
-	if (xr->kvrd == NULL)
-		goto fail;
-
 	return xr;
-fail:
-	sqfs_destroy(xr);
-	return NULL;
 }
diff --git a/tar/sqfs2tar.c b/tar/sqfs2tar.c
index 5a786dd..2e4d0ef 100644
--- a/tar/sqfs2tar.c
+++ b/tar/sqfs2tar.c
@@ -611,14 +611,14 @@ int main(int argc, char **argv)
 	}
 
 	if (!no_xattr && !(super.flags & SQFS_FLAG_NO_XATTRS)) {
-		xr = sqfs_xattr_reader_create(file, &super, cmp);
+		xr = sqfs_xattr_reader_create(0);
 		if (xr == NULL) {
 			sqfs_perror(filename, "creating xattr reader",
 				    SQFS_ERROR_ALLOC);
 			goto out_dr;
 		}
 
-		ret = sqfs_xattr_reader_load_locations(xr);
+		ret = sqfs_xattr_reader_load(xr, &super, file, cmp);
 		if (ret) {
 			sqfs_perror(filename, "loading xattr table", ret);
 			goto out_xr;
diff --git a/unpack/rdsquashfs.c b/unpack/rdsquashfs.c
index 99e51fb..7b3bddf 100644
--- a/unpack/rdsquashfs.c
+++ b/unpack/rdsquashfs.c
@@ -74,14 +74,14 @@ int main(int argc, char **argv)
 	}
 
 	if (!(super.flags & SQFS_FLAG_NO_XATTRS)) {
-		xattr = sqfs_xattr_reader_create(file, &super, cmp);
+		xattr = sqfs_xattr_reader_create(0);
 		if (xattr == NULL) {
 			sqfs_perror(opt.image_name, "creating xattr reader",
 				    SQFS_ERROR_ALLOC);
 			goto out_cmp;
 		}
 
-		ret = sqfs_xattr_reader_load_locations(xattr);
+		ret = sqfs_xattr_reader_load(xattr, &super, file, cmp);
 		if (ret) {
 			sqfs_perror(opt.image_name, "loading xattr table",
 				    ret);