libflash/libffs: Don't require 'part' size to be known by callers

Currently the FFS header/TOC generation code requires that consumers
know the size of their TOC beforehand. While this may be advantageous in
some circumstances if there are known limitations of other software. It
should not be a requirement.

Knowing the size of the FFS header/TOC partially breaks the abstraction
since it would require consumers of the library to be aware of/have some
idea of the on flash structure and size.

Future work may introduce functions to force sizes but the default
behaviour should be to calculate it behind the scenes.

This patch also addresses an off by one issue in checking for TOC
overflow.

Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Reviewed-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

13 files changed, 37 insertions, 35 deletions

diff --git a/external/ffspart/ffspart.c b/external/ffspart/ffspart.c
index 50b5861..7703477 100644
--- a/external/ffspart/ffspart.c
+++ b/external/ffspart/ffspart.c
@@ -174,17 +174,11 @@ int main(int argc, char *argv[])
 		goto out;
 	}
 
-	/*
-	 * TODO: Rethink, is ffspart providing a variable TOC size useful?
-	 * Use 1 block for the size of the partition table...
-	 */
-	rc = ffs_hdr_new(block_size, block_size, block_count / sides, &new_hdr);
+	rc = ffs_hdr_new(block_size, block_count / sides, &new_hdr);
 	if (rc) {
 		if (rc == FFS_ERR_BAD_SIZE) {
 			/* Well this check is a tad redudant now */
-			fprintf(stderr, "Bad size parametres passed to libffs: "
-					"size must be a multiple of block_size\n"
-					"size (%u), block_size (%u) \n", block_size, block_size);
+			fprintf(stderr, "Bad parametres passed to libffs\n");
 		} else {
 			fprintf(stderr, "Error %d initialising new TOC\n", rc);
 		}
diff --git a/external/ffspart/test/files/03-tiny-pnor.in b/external/ffspart/test/files/03-tiny-pnor.in
index 517dc47..3c02b02 100644
--- a/external/ffspart/test/files/03-tiny-pnor.in
+++ b/external/ffspart/test/files/03-tiny-pnor.in
@@ -1,4 +1,4 @@
-ONE,0x00000300,0x00000100,EV,/dev/zero
-TWO,0x00000400,0x00000100,EF,/dev/zero
-THREE,0x00000500,0x00000100,EF,/dev/zero
-FOUR,0x00000600,0x00000100,EF,/dev/zero
+ONE,0x00400,0x00000100,EV,/dev/zero
+TWO,0x00500,0x00000100,EF,/dev/zero
+THREE,0x600,0x00000100,EF,/dev/zero
+FOUR,0x0700,0x00000100,EF,/dev/zero
diff --git a/external/ffspart/test/files/03-tiny-pnor.out b/external/ffspart/test/files/03-tiny-pnor.out
index 32e998d..e00fa5c 100644
--- a/external/ffspart/test/files/03-tiny-pnor.out
+++ b/external/ffspart/test/files/03-tiny-pnor.out
diff --git a/external/ffspart/test/files/03.1-tiny-pnor-backup.in b/external/ffspart/test/files/03.1-tiny-pnor-backup.in
index 517dc47..b552750 100644
--- a/external/ffspart/test/files/03.1-tiny-pnor-backup.in
+++ b/external/ffspart/test/files/03.1-tiny-pnor-backup.in
@@ -1,4 +1,4 @@
-ONE,0x00000300,0x00000100,EV,/dev/zero
-TWO,0x00000400,0x00000100,EF,/dev/zero
-THREE,0x00000500,0x00000100,EF,/dev/zero
-FOUR,0x00000600,0x00000100,EF,/dev/zero
+ONE,0x00400,0x100,EV,/dev/zero
+TWO,0x00500,0x100,EF,/dev/zero
+THREE,0x600,0x100,EF,/dev/zero
+FOUR,0x0700,0x100,EF,/dev/zero
diff --git a/external/ffspart/test/files/03.1-tiny-pnor-backup.out b/external/ffspart/test/files/03.1-tiny-pnor-backup.out
index 85c23e3..e173e9e 100644
--- a/external/ffspart/test/files/03.1-tiny-pnor-backup.out
+++ b/external/ffspart/test/files/03.1-tiny-pnor-backup.out
diff --git a/external/ffspart/test/files/04-tiny-pnor2.out b/external/ffspart/test/files/04-tiny-pnor2.out
index dad94b8..394edf0 100644
--- a/external/ffspart/test/files/04-tiny-pnor2.out
+++ b/external/ffspart/test/files/04-tiny-pnor2.out
diff --git a/external/ffspart/test/results/05-hdr-overlap.err b/external/ffspart/test/results/05-hdr-overlap.err
index c0ab238..55f07c6 100644
--- a/external/ffspart/test/results/05-hdr-overlap.err
+++ b/external/ffspart/test/results/05-hdr-overlap.err
@@ -1,2 +1,2 @@
-Adding partition 'FOUR' would cause partition 'ONE' at 0x00000200 to overlap with the header
-Couldn't add entry 'FOUR' 0x00000500 for 0x00000100
+Adding partition 'THREE' would cause partition 'ONE' at 0x00000200 to overlap with the header
+Couldn't add entry 'THREE' 0x00000400 for 0x00000100
diff --git a/external/ffspart/test/results/05-hdr-overlap.out b/external/ffspart/test/results/05-hdr-overlap.out
index 9613697..dbcdcb1 100644
--- a/external/ffspart/test/results/05-hdr-overlap.out
+++ b/external/ffspart/test/results/05-hdr-overlap.out
@@ -1,5 +1,4 @@
 Adding 'ONE' 0x00000200, 0x00000100
 Adding 'TWO' 0x00000300, 0x00000100
 Adding 'THREE' 0x00000400, 0x00000100
-Adding 'FOUR' 0x00000500, 0x00000100
 Freeing hdr
diff --git a/external/ffspart/test/results/05.1-hdr-overlap-backup.err b/external/ffspart/test/results/05.1-hdr-overlap-backup.err
index 2adbf79..55f07c6 100644
--- a/external/ffspart/test/results/05.1-hdr-overlap-backup.err
+++ b/external/ffspart/test/results/05.1-hdr-overlap-backup.err
@@ -1,2 +1,2 @@
-Adding partition 'BACKUP_PART' would cause partition 'ONE' at 0x00000200 to overlap with the header
-Failed to create backup part
+Adding partition 'THREE' would cause partition 'ONE' at 0x00000200 to overlap with the header
+Couldn't add entry 'THREE' 0x00000400 for 0x00000100
diff --git a/external/ffspart/test/tests/03.1-tiny-pnor-backup b/external/ffspart/test/tests/03.1-tiny-pnor-backup
index a622ca6..3065c86 100644
--- a/external/ffspart/test/tests/03.1-tiny-pnor-backup
+++ b/external/ffspart/test/tests/03.1-tiny-pnor-backup
@@ -2,7 +2,7 @@
 
 touch $DATA_DIR/$CUR_TEST.gen
 
-run_binary "./ffspart" "-b -s 0x100 -c 10 -i $DATA_DIR/$CUR_TEST.in -p $DATA_DIR/$CUR_TEST.gen"
+run_binary "./ffspart" "-b -s 0x100 -c 15 -i $DATA_DIR/$CUR_TEST.in -p $DATA_DIR/$CUR_TEST.gen"
 if [ "$?" -ne 0 ] ; then
 	fail_test
 fi
diff --git a/libflash/ffs.h b/libflash/ffs.h
index 6ba7713..1872253 100644
--- a/libflash/ffs.h
+++ b/libflash/ffs.h
@@ -213,6 +213,7 @@ struct ffs_hdr {
 	uint32_t size;
 	uint32_t block_size;
 	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 4f9509b..038f594 100644
--- a/libflash/libffs.c
+++ b/libflash/libffs.c
@@ -490,7 +490,7 @@ static int __ffs_entry_add(struct ffs_hdr *hdr, struct ffs_entry *entry)
 		count++;
 	}
 
-	if (count * sizeof(struct __ffs_entry) +
+	if ((count + 1) * sizeof(struct __ffs_entry) +
 			sizeof(struct __ffs_hdr) > smallest_base) {
 		fprintf(stderr, "Adding partition '%s' would cause partition '%s' at "
 				"0x%08x to overlap with the header\n", entry->name, smallest_name,
@@ -569,15 +569,19 @@ int ffs_hdr_create_backup(struct ffs_hdr *hdr)
 {
 	struct ffs_entry *ent;
 	struct ffs_entry *backup;
+	uint32_t hdr_size, flash_end;
 	int rc = 0;
+
 	ent = list_tail(&hdr->entries, struct ffs_entry, list);
 	if (!ent) {
 		return FLASH_ERR_PARM_ERROR;
 	}
 
-	rc = ffs_entry_new("BACKUP_PART",
-		hdr->base + (hdr->block_size * (hdr->block_count - 1 )) - hdr->size,
-		hdr->size, &backup);
+	hdr_size = ffs_hdr_raw_size(ffs_num_entries(hdr) + 1);
+	/* Whole number of blocks BACKUP_PART needs to be */
+	hdr_size = ((hdr_size + hdr->block_size) / hdr->block_size) * hdr->block_size;
+	flash_end = hdr->base + (hdr->block_size * hdr->block_count);
+	rc = ffs_entry_new("BACKUP_PART", flash_end - hdr_size, hdr_size, &backup);
 	if (rc)
 		return rc;
 
@@ -604,7 +608,7 @@ int ffs_hdr_add_side(struct ffs_hdr *hdr)
 	if (hdr->side)
 		return FLASH_ERR_PARM_ERROR;
 
-	rc = ffs_hdr_new(hdr->size, hdr->block_size, hdr->block_count, &hdr->side);
+	rc = ffs_hdr_new(hdr->block_size, hdr->block_count, &hdr->side);
 	if (rc)
 		return rc;
 
@@ -649,9 +653,15 @@ int ffs_hdr_finalise(struct blocklevel_device *bl, struct ffs_hdr *hdr)
 	 */
 	memset(real_hdr, 0, sizeof(*real_hdr));
 
+	hdr->part->size = ffs_hdr_raw_size(num_entries) + hdr->block_size;
+	/*
+	 * So actual is in bytes. ffs_entry_to_flash() don't do the
+	 * block_size division that we're relying on
+	 */
+	hdr->part->actual = (hdr->part->size / hdr->block_size) * hdr->block_size;
 	real_hdr->magic = cpu_to_be32(FFS_MAGIC);
 	real_hdr->version = cpu_to_be32(hdr->version);
-	real_hdr->size = cpu_to_be32(hdr->size / hdr->block_size);
+	real_hdr->size = cpu_to_be32(hdr->part->size / hdr->block_size);
 	real_hdr->entry_size = cpu_to_be32(sizeof(struct __ffs_entry));
 	real_hdr->entry_count = cpu_to_be32(num_entries);
 	real_hdr->block_size = cpu_to_be32(hdr->block_size);
@@ -740,30 +750,28 @@ int ffs_entry_new(const char *name, uint32_t base, uint32_t size, struct ffs_ent
 	return 0;
 }
 
-int ffs_hdr_new(uint32_t size, uint32_t block_size, uint32_t block_count, struct ffs_hdr **r)
+int ffs_hdr_new(uint32_t block_size, uint32_t block_count, struct ffs_hdr **r)
 {
 	struct ffs_hdr *ret;
 	struct ffs_entry *part_table;
 	int rc;
 
-	if (size % block_size || size > block_size * block_count)
-		return FFS_ERR_BAD_SIZE;
-
 	ret = calloc(1, sizeof(*ret));
 	if (!ret)
 		return FLASH_ERR_MALLOC_FAILED;
 
 	ret->version = FFS_VERSION_1;
-	ret->size = size;
 	ret->block_size = block_size;
 	ret->block_count = block_count;
 	list_head_init(&ret->entries);
 
-	rc = ffs_entry_new("part", 0, size, &part_table);
+	/* Don't know how big it will be, ffs_hdr_finalise() will fix */
+	rc = ffs_entry_new("part", 0, 0, &part_table);
 	if (rc) {
 		free(ret);
 		return rc;
 	}
+	ret->part = part_table;
 
 	part_table->pid = FFS_PID_TOPLEVEL;
 	part_table->type = FFS_TYPE_PARTITION;
diff --git a/libflash/libffs.h b/libflash/libffs.h
index a86c698..a0f65a0 100644
--- a/libflash/libffs.h
+++ b/libflash/libffs.h
@@ -133,7 +133,7 @@ struct ffs_entry *ffs_entry_get(struct ffs_handle *ffs, uint32_t index);
 int ffs_update_act_size(struct ffs_handle *ffs, uint32_t part_idx,
 			uint32_t act_size);
 
-int ffs_hdr_new(uint32_t size, uint32_t block_size, uint32_t block_count,
+int ffs_hdr_new(uint32_t block_size, uint32_t block_count,
 		struct ffs_hdr **r);
 
 int ffs_hdr_add_side(struct ffs_hdr *hdr);