fiptool: Embed a pointer to an image within the image descriptor
Currently, fiptool uses two linked lists. One to chain together all
the images and one for all the image descriptors. Initially this was
done because not all images had a corresponding image descriptor.
This was the case for unknown images which existed in the FIP but
there was no descriptor in the builtin table for them. When support
for the --blob option came in, we started building descriptors for the
unknown images on the fly. As a result every image now has a
corresponding image descriptor and therefore it is no longer necessary
to keep track of them separately.
To simplify the design, maintain only a single linked list of image
descriptors. An image descriptor contains a pointer to the
corresponding image. If the pointer is NULL, then the descriptor is
skipped in all the operations. This approach simplifies the traversal
code and avoids redundant lookups.
The linked list of image descriptors is populated based on the
`toc_entries` array. This means that the order of the images in the
FIP file remains the same across add/remove or create/update
operations. This is true for all standard images (those specified in
`toc_entries`) but not for those specified via the --blob option.
Change-Id: Ic29a263c86c8f1efdad322b430368c7623782e2d
Signed-off-by: dp-arm <dimitris.papastamos@arm.com>
diff --git a/tools/fiptool/fiptool.c b/tools/fiptool/fiptool.c
index f3f831b..542a946 100644
--- a/tools/fiptool/fiptool.c
+++ b/tools/fiptool/fiptool.c
@@ -80,8 +80,6 @@
static image_desc_t *image_desc_head;
static size_t nr_image_descs;
-static image_t *image_head;
-static size_t nr_images;
static uuid_t uuid_null = { 0 };
static int verbose;
@@ -200,6 +198,7 @@
free(desc->name);
free(desc->cmdline_name);
free(desc->action_arg);
+ free(desc->image);
free(desc);
}
@@ -244,74 +243,6 @@
}
}
-static void add_image(image_t *image)
-{
- image_t **p = &image_head;
-
- while (*p)
- p = &(*p)->next;
-
- assert(*p == NULL);
- *p = image;
-
- nr_images++;
-}
-
-static void replace_image(image_t *image)
-{
- image_t **p = &image_head;
-
- while (*p) {
- if (!memcmp(&(*p)->toc_e.uuid, &image->toc_e.uuid,
- sizeof(image->toc_e.uuid)))
- break;
- p = &(*p)->next;
- }
-
- assert(*p != NULL);
-
- image->next = (*p)->next;
- *p = image;
-}
-
-static void free_image(image_t *image)
-{
- free(image->buffer);
- free(image);
-}
-
-static void remove_image(image_t *image)
-{
- image_t *tmp, **p = &image_head;
-
- while (*p) {
- if (*p == image)
- break;
- p = &(*p)->next;
- }
-
- assert(*p != NULL);
-
- tmp = *p;
- *p = tmp->next;
- free_image(tmp);
-
- nr_images--;
-}
-
-static void free_images(void)
-{
- image_t *image = image_head, *tmp;
-
- while (image != NULL) {
- tmp = image->next;
- free_image(image);
- image = tmp;
- nr_images--;
- }
- assert(nr_images == 0);
-}
-
static image_desc_t *lookup_image_desc_from_uuid(const uuid_t *uuid)
{
image_desc_t *desc;
@@ -332,16 +263,6 @@
return NULL;
}
-static image_t *lookup_image_from_uuid(const uuid_t *uuid)
-{
- image_t *image;
-
- for (image = image_head; image != NULL; image = image->next)
- if (!memcmp(&image->toc_e.uuid, uuid, sizeof(*uuid)))
- return image;
- return NULL;
-}
-
static void uuid_to_str(char *s, size_t len, const uuid_t *u)
{
assert(len >= (_UUID_STR_LEN + 1));
@@ -457,7 +378,8 @@
add_image_desc(desc);
}
- add_image(image);
+ assert(desc->image == NULL);
+ desc->image = image;
toc_entry++;
}
@@ -542,7 +464,7 @@
static int info_cmd(int argc, char *argv[])
{
- image_t *image;
+ image_desc_t *desc;
fip_toc_header_t toc_header;
if (argc != 2)
@@ -560,11 +482,11 @@
(unsigned long long)toc_header.flags);
}
- for (image = image_head; image != NULL; image = image->next) {
- image_desc_t *desc;
+ for (desc = image_desc_head; desc != NULL; desc = desc->next) {
+ image_t *image = desc->image;
- desc = lookup_image_desc_from_uuid(&image->toc_e.uuid);
- assert(desc != NULL);
+ if (image == NULL)
+ continue;
printf("%s: offset=0x%llX, size=0x%llX, cmdline=\"--%s\"",
desc->name,
(unsigned long long)image->toc_e.offset_address,
@@ -580,7 +502,6 @@
putchar('\n');
}
- free_images();
return 0;
}
@@ -593,11 +514,16 @@
static int pack_images(const char *filename, uint64_t toc_flags, unsigned long align)
{
FILE *fp;
- image_t *image;
+ image_desc_t *desc;
fip_toc_header_t *toc_header;
fip_toc_entry_t *toc_entry;
char *buf;
uint64_t entry_offset, buf_size, payload_size = 0;
+ size_t nr_images = 0;
+
+ for (desc = image_desc_head; desc != NULL; desc = desc->next)
+ if (desc->image != NULL)
+ nr_images++;
buf_size = sizeof(fip_toc_header_t) +
sizeof(fip_toc_entry_t) * (nr_images + 1);
@@ -614,7 +540,11 @@
toc_entry = (fip_toc_entry_t *)(toc_header + 1);
entry_offset = buf_size;
- for (image = image_head; image != NULL; image = image->next) {
+ for (desc = image_desc_head; desc != NULL; desc = desc->next) {
+ image_t *image = desc->image;
+
+ if (image == NULL)
+ continue;
payload_size += image->toc_e.size;
entry_offset = (entry_offset + align - 1) & ~(align - 1);
image->toc_e.offset_address = entry_offset;
@@ -640,7 +570,11 @@
if (verbose)
log_dbgx("Payload size: %zu bytes", payload_size);
- for (image = image_head; image != NULL; image = image->next) {
+ for (desc = image_desc_head; desc != NULL; desc = desc->next) {
+ image_t *image = desc->image;
+
+ if (image == NULL)
+ continue;
if (fseek(fp, image->toc_e.offset_address, SEEK_SET))
log_errx("Failed to set file position");
@@ -664,26 +598,26 @@
/* Add or replace images in the FIP file. */
for (desc = image_desc_head; desc != NULL; desc = desc->next) {
- image_t *new_image, *old_image;
+ image_t *image;
if (desc->action != DO_PACK)
continue;
- new_image = read_image_from_file(&desc->uuid,
+ image = read_image_from_file(&desc->uuid,
desc->action_arg);
- old_image = lookup_image_from_uuid(&desc->uuid);
- if (old_image != NULL) {
+ if (desc->image != NULL) {
if (verbose) {
log_dbgx("Replacing %s with %s",
desc->cmdline_name,
desc->action_arg);
}
- replace_image(new_image);
+ free(desc->image);
+ desc->image = image;
} else {
if (verbose)
log_dbgx("Adding image %s",
desc->action_arg);
- add_image(new_image);
+ desc->image = image;
}
}
}
@@ -808,7 +742,6 @@
update_fip();
pack_images(argv[0], toc_flags, align);
- free_images();
return 0;
}
@@ -922,7 +855,6 @@
update_fip();
pack_images(outfile, toc_flags, align);
- free_images();
return 0;
}
@@ -1028,7 +960,7 @@
/* Unpack all specified images. */
for (desc = image_desc_head; desc != NULL; desc = desc->next) {
char file[PATH_MAX];
- image_t *image;
+ image_t *image = desc->image;
if (!unpack_all && desc->action != DO_UNPACK)
continue;
@@ -1041,7 +973,6 @@
snprintf(file, sizeof(file), "%s",
desc->action_arg);
- image = lookup_image_from_uuid(&desc->uuid);
if (image == NULL) {
if (!unpack_all)
log_warnx("%s does not exist in %s",
@@ -1059,7 +990,6 @@
}
}
- free_images();
return 0;
}
@@ -1168,17 +1098,15 @@
parse_fip(argv[0], &toc_header);
for (desc = image_desc_head; desc != NULL; desc = desc->next) {
- image_t *image;
-
if (desc->action != DO_REMOVE)
continue;
- image = lookup_image_from_uuid(&desc->uuid);
- if (image != NULL) {
+ if (desc->image != NULL) {
if (verbose)
log_dbgx("Removing %s",
desc->cmdline_name);
- remove_image(image);
+ free(desc->image);
+ desc->image = NULL;
} else {
log_warnx("%s does not exist in %s",
desc->cmdline_name, argv[0]);
@@ -1186,7 +1114,6 @@
}
pack_images(outfile, toc_header.flags, align);
- free_images();
return 0;
}
diff --git a/tools/fiptool/fiptool.h b/tools/fiptool/fiptool.h
index be0c6f0..c4c86bc 100644
--- a/tools/fiptool/fiptool.h
+++ b/tools/fiptool/fiptool.h
@@ -58,13 +58,13 @@
char *cmdline_name;
int action;
char *action_arg;
+ struct image *image;
struct image_desc *next;
} image_desc_t;
typedef struct image {
struct fip_toc_entry toc_e;
void *buffer;
- struct image *next;
} image_t;
typedef struct cmd {