From ea4d4c9c337ec51096a2e1e12c0de5919719265b Mon Sep 17 00:00:00 2001 From: Julius Werner Date: Tue, 11 Apr 2023 17:01:00 -0700 Subject: [PATCH] cbfstool: Add comment to define stability rules for cbfstool print -k In CB:41119, I sort of made up a mechanism on the fly for how to make the machine-parseable cbfstool print output extensible without breaking backwards compatibility for older scripts. But I only explained it in the commit message which is not very visible. This patch adds a comment to the function that generates that output so that people who want to change it can understand the intent. Signed-off-by: Julius Werner Change-Id: I0d18d59e7fe407eb34710d6a583cfae667723eb7 Reviewed-on: https://review.coreboot.org/c/coreboot/+/74347 Tested-by: build bot (Jenkins) Reviewed-by: Yu-Ping Wu --- util/cbfstool/cbfs_image.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index 683a96cac7..7a34bfe58f 100644 --- a/util/cbfstool/cbfs_image.c +++ b/util/cbfstool/cbfs_image.c @@ -730,6 +730,12 @@ int cbfs_add_entry(struct cbfs_image *image, struct buffer *buffer, const char *name = header->filename; + /* This is so special rows in cbfstool print -k -v output stay unambiguous. */ + if (name[0] == '[') { + ERROR("CBFS file name `%s` must not start with `[`\n", name); + return -1; + } + uint32_t entry_type; uint32_t addr, addr_next; struct cbfs_file *entry, *next; @@ -1497,6 +1503,32 @@ int cbfs_print_entry_info(struct cbfs_image *image, struct cbfs_file *entry, return 0; } +/* + * The format of this output has been stable for many years. Since it is meant + * to be parsed by scripts, we should probably not lightly make changes to it as + * that could break older scripts expecting a different format. + * + * Until CB:41119, the `-v` flag made no difference when `-k` was selected, so + * presumably no scripts were using that combination. That's why that patch left + * the output for `-k` by itself alone to avoid breaking legacy scripts, and + * expanded `-k -v` to allow an arbitrary number of `:` tokens at + * the end of each row behind the legacy column output. So the new output format + * stability rules should be that `-k` will stay as it is, and `-k -v` may be + * expanded to add more `:` tokens to the end of a row. Scripts that + * want to parse `-k -v` output should be written to gracefully ignore any extra + * such tokens where they don't recognize the key. + * + * The `-k -v` output may also include extra rows that start with a `[`. These + * do not represent a CBFS file and can instead be used to display data that is + * associated with the CBFS as a whole and not any single file. Currently + * defined are `[FMAP REGION]\t` and + * `[METADATA HASH]\t:`. More may be defined in the future and + * scripts parsing `-k -v` output should be written to gracefully ignore any + * rows starting with `[` that they don't recognize. + * + * The format for existing `` tokens or `[` rows should never be + * changed once they are added. + */ static int cbfs_print_parseable_entry_info(struct cbfs_image *image, struct cbfs_file *entry, void *arg) {