[Skiboot] [RFC PATCH 08/13] libflash/libffs: Remove the 'sides' from the FFS TOC generation code

Cyril Bur cyril.bur at au1.ibm.com
Tue Aug 29 16:25:01 AEST 2017


It turns out this code was messy and not all that reliable. Doing it at
the library level adds complexity to the library and restrictions to the
caller.

A simpler approach can be achived with the just instantiating multiple
ffs_header structures pointing to different parts of the same file.

Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
---
 external/ffspart/ffspart.c |  2 +-
 libflash/ffs.h             |  2 --
 libflash/libffs.c          | 77 ++--------------------------------------------
 libflash/libffs.h          |  3 +-
 4 files changed, 5 insertions(+), 79 deletions(-)

diff --git a/external/ffspart/ffspart.c b/external/ffspart/ffspart.c
index bd223c78..74e12ff6 100644
--- a/external/ffspart/ffspart.c
+++ b/external/ffspart/ffspart.c
@@ -273,7 +273,7 @@ int main(int argc, char *argv[])
 			goto out_while;
 		}
 
-		rc = ffs_entry_add(new_hdr, new_entry, 0);
+		rc = ffs_entry_add(new_hdr, new_entry);
 		if (rc) {
 			fprintf(stderr, "Couldn't add entry '%s' 0x%08x for 0x%08x\n",
 					name, pbase, psize);
diff --git a/libflash/ffs.h b/libflash/ffs.h
index 81818fdf..cc2460c8 100644
--- a/libflash/ffs.h
+++ b/libflash/ffs.h
@@ -205,7 +205,6 @@ struct __ffs_hdr {
  * @block_size:		Size of block on device (in bytes)
  * @block_count:	Number of blocks on device.
  * @backup		The backup partition
- * @side		The ffs header for the other side
  * @entries:		List of partition entries
  */
 struct ffs_hdr {
@@ -216,7 +215,6 @@ struct ffs_hdr {
 	uint32_t block_count;
 	struct ffs_entry *part;
 	struct ffs_entry *backup;
-	struct ffs_hdr *side;
 	struct list_head entries;
 };
 
diff --git a/libflash/libffs.c b/libflash/libffs.c
index 0159c07b..e741121d 100644
--- a/libflash/libffs.c
+++ b/libflash/libffs.c
@@ -436,10 +436,6 @@ static void __hdr_free(struct ffs_hdr *hdr)
 		list_del(&ent->list);
 		free(ent);
 	}
-	if (hdr->side) {
-		hdr->side->side = NULL;
-		ffs_hdr_free(hdr->side);
-	}
 }
 
 int ffs_hdr_free(struct ffs_hdr *hdr)
@@ -603,10 +599,8 @@ static int __ffs_entry_add(struct ffs_hdr *hdr, struct ffs_entry *entry)
 	return 0;
 }
 
-int ffs_entry_add(struct ffs_hdr *hdr, struct ffs_entry *entry, unsigned int side)
+int ffs_entry_add(struct ffs_hdr *hdr, struct ffs_entry *entry)
 {
-	int rc;
-
 	/*
 	 * Refuse to add anything after BACKUP_PART has been added, not
 	 * sure why this is needed anymore
@@ -614,36 +608,7 @@ int ffs_entry_add(struct ffs_hdr *hdr, struct ffs_entry *entry, unsigned int sid
 	if (hdr->backup)
 		return FLASH_ERR_PARM_ERROR;
 
-	if (side == 0) { /* Sideless... */
-		rc = __ffs_entry_add(hdr, entry);
-		if (!rc && hdr->side) {
-			struct ffs_entry *other_ent;
-
-			/*
-			 * A rather sneaky copy is hidden here.
-			 * It doesn't make sense for a consumer to be aware that structures
-			 * must be duplicated. The entries list in the header could have
-			 * been an array of pointers and no copy would have been required.
-			 */
-			other_ent = calloc(1, sizeof (struct ffs_entry));
-			if (!other_ent)
-				/* TODO Remove the added entry from side 1 */
-				return FLASH_ERR_PARM_ERROR;
-			memcpy(other_ent, entry, sizeof(struct ffs_entry));
-			rc = __ffs_entry_add(hdr->side, other_ent);
-			if (rc)
-				/* TODO Remove the added entry from side 1 */
-				free(other_ent);
-		}
-	} else if (side == 1) {
-		rc = __ffs_entry_add(hdr, entry);
-	} else if (side == 2 && hdr->side) {
-		rc = __ffs_entry_add(hdr->side, entry);
-	} else {
-		rc = FLASH_ERR_PARM_ERROR;
-	}
-
-	return rc;
+	return __ffs_entry_add(hdr, entry);
 }
 
 /* This should be done last! */
@@ -675,29 +640,6 @@ int ffs_hdr_create_backup(struct ffs_hdr *hdr)
 
 	hdr->backup = backup;
 
-	/* Do we try to roll back completely if that fails or leave what we've added? */
-	if (hdr->side && hdr->base == 0)
-		rc = ffs_hdr_create_backup(hdr->side);
-
-	return rc;
-}
-
-int ffs_hdr_add_side(struct ffs_hdr *hdr)
-{
-	int rc;
-
-	/* Only a second side for now */
-	if (hdr->side)
-		return FLASH_ERR_PARM_ERROR;
-
-	rc = ffs_hdr_new(hdr->block_size, hdr->block_count, &hdr->side);
-	if (rc)
-		return rc;
-
-	hdr->side->base = hdr->block_size * hdr->block_count;
-	/* Sigh */
-	hdr->side->side = hdr;
-
 	return rc;
 }
 
@@ -713,16 +655,6 @@ int ffs_hdr_finalise(struct blocklevel_device *bl, struct ffs_hdr *hdr)
 	if (num_entries == 0)
 		return FFS_ERR_BAD_SIZE;
 
-	if (hdr->side) {
-		struct ffs_entry *other_side;
-		/* TODO: Change the hard coded 0x8000 */
-		rc = ffs_entry_new("OTHER_SIDE", hdr->side->base, 0x8000, &other_side);
-		if (rc)
-			return rc;
-		list_add_tail(&hdr->entries, &other_side->list);
-		num_entries++;
-	}
-
 	real_hdr = malloc(ffs_hdr_raw_size(num_entries));
 	if (!real_hdr)
 		return FLASH_ERR_MALLOC_FAILED;
@@ -773,11 +705,6 @@ int ffs_hdr_finalise(struct blocklevel_device *bl, struct ffs_hdr *hdr)
 		rc = blocklevel_write(bl, hdr->backup->base, real_hdr,
 			ffs_hdr_raw_size(num_entries));
 	}
-	if (rc)
-		goto out;
-
-	if (hdr->side && hdr->base == 0)
-		rc = ffs_hdr_finalise(bl, hdr->side);
 out:
 	free(real_hdr);
 	return rc;
diff --git a/libflash/libffs.h b/libflash/libffs.h
index 2196d526..d81b9dfe 100644
--- a/libflash/libffs.h
+++ b/libflash/libffs.h
@@ -149,10 +149,11 @@ int ffs_entry_user_set(struct ffs_entry *ent, struct ffs_entry_user *user);
 
 int ffs_entry_set_act_size(struct ffs_entry *ent, uint32_t actual_size);
 
-int ffs_entry_add(struct ffs_hdr *hdr, struct ffs_entry *entry, unsigned int side);
 
 struct ffs_entry_user ffs_entry_user_get(struct ffs_entry *ent);
 
+int ffs_entry_add(struct ffs_hdr *hdr, struct ffs_entry *entry);
+
 int ffs_hdr_create_backup(struct ffs_hdr *hdr);
 
 int ffs_hdr_finalise(struct blocklevel_device *bl, struct ffs_hdr *hdr);
-- 
2.14.1



More information about the Skiboot mailing list