Re: [PATCH v2 2/4] tools/bootconfig: Consolidate xbc_init() to error message wrapper

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Use init_xbc_with_error() for all bootconfig initialization in the
bootconfig tool instead of showing errors in different way.

This simplifies the code logic and make it easy to maintain.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v2:
 - Remove redundant buffer copy in init_xbc_with_error().
---
 tools/bootconfig/main.c |  100 ++++++++++++++++++++---------------------------
 1 file changed, 43 insertions(+), 57 deletions(-)

diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c
index aff169ba75b8..652e491b9c33 100644
--- a/tools/bootconfig/main.c
+++ b/tools/bootconfig/main.c

@@ -21,6 +21,39 @@
 #define BOOTCONFIG_FOOTER_SIZE \
        (sizeof(uint32_t) * 2 + BOOTCONFIG_MAGIC_LEN)

+static void show_xbc_error(const char *data, const char *msg, int pos)
+{
+       int lin = 1, col, i;
+
+       if (pos < 0) {
+               pr_err("Error: %s.\n", msg);
+               return;
+       }
+
+       /* Note that pos starts from 0 but lin and col should start from 1. */
+       col = pos + 1;
+       for (i = 0; i < pos; i++) {
+               if (data[i] == '\n') {
+                       lin++;
+                       col = pos - i;
+               }
+       }
+       pr_err("Parse Error: %s at %d:%d\n", msg, lin, col);
+
+}
+
+static int init_xbc_with_error(char *buf, int len)
+{
+       const char *msg;
+       int ret, pos;
+
+       ret = xbc_init(buf, len, &msg, &pos);
+       if (ret < 0)
+               show_xbc_error(buf, msg, pos);
+
+       return ret;
+}
+
 static int xbc_show_value(struct xbc_node *node, bool semicolon)
 {
        const char *val, *eol;

@@ -197,7 +230,6 @@ static int load_xbc_from_initrd(int fd, char **buf)
        int ret;
        uint32_t size = 0, csum = 0, rcsum;
        char magic[BOOTCONFIG_MAGIC_LEN];
-       const char *msg;

        ret = fstat(fd, &stat);
        if (ret < 0)

@@ -249,52 +281,9 @@ static int load_xbc_from_initrd(int fd, char **buf)
                return -EINVAL;
        }

-       ret = xbc_init(*buf, size, &msg, NULL);
-       /* Wrong data */
-       if (ret < 0) {
-               pr_err("parse error: %s.\n", msg);
-               return ret;
-       }
-
-       return size;
-}
-
-static void show_xbc_error(const char *data, const char *msg, int pos)
-{
-       int lin = 1, col, i;
-
-       if (pos < 0) {
-               pr_err("Error: %s.\n", msg);
-               return;
-       }
-
-       /* Note that pos starts from 0 but lin and col should start from 1. */
-       col = pos + 1;
-       for (i = 0; i < pos; i++) {
-               if (data[i] == '\n') {
-                       lin++;
-                       col = pos - i;
-               }
-       }
-       pr_err("Parse Error: %s at %d:%d\n", msg, lin, col);
+       ret = init_xbc_with_error(*buf, size);

-}
-
-static int init_xbc_with_error(char *buf, int len)
-{
-       char *copy = strdup(buf);
-       const char *msg;
-       int ret, pos;
-
-       if (!copy)
-               return -ENOMEM;
-
-       ret = xbc_init(buf, len, &msg, &pos);
-       if (ret < 0)
-               show_xbc_error(copy, msg, pos);
-       free(copy);
-
-       return ret;
+       return ret < 0 ? ret : size;
 }

 static int show_xbc_kernel_cmdline(void)

@@ -423,9 +412,8 @@ static int apply_xbc(const char *path, const char *xbc_path)
        char *buf, *data;
        size_t total_size;
        struct stat stat;
-       const char *msg;
        uint32_t size, csum;
-       int pos, pad;
+       int pad;
        int ret, fd;

        ret = load_xbc_file(xbc_path, &buf);

@@ -438,6 +426,13 @@ static int apply_xbc(const char *path, const char *xbc_path)
                size++;
        csum = xbc_calc_checksum(buf, size);

+       /* Verify the data format */
+       ret = init_xbc_with_error(buf, size);
+       if (ret < 0) {
+               free(buf);
+               return ret;
+       }
+
        /* Backup the bootconfig data */
        data = calloc(size + BOOTCONFIG_ALIGN + BOOTCONFIG_FOOTER_SIZE, 1);
        if (!data) {

@@ -446,15 +441,6 @@ static int apply_xbc(const char *path, const char *xbc_path)
        }
        memcpy(data, buf, size);

-       /* Check the data format */
-       ret = xbc_init(buf, size, &msg, &pos);
-       if (ret < 0) {
-               show_xbc_error(data, msg, pos);
-               free(data);
-               free(buf);
-
-               return ret;
-       }
        printf("Apply %s to %s\n", xbc_path, path);
        xbc_get_info(&ret, NULL);
        printf("\tNumber of nodes: %d\n", ret);