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

[Sang-Heon Jeon]

On Fri, Sep 11, 2026 at 11:13 PM Masami Hiramatsu (Google)
<mhiramat@xxxxxxxxxx> wrote:
>
> From: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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);
>

Reviewed-by: Sang-Heon Jeon <ekffu200098@xxxxxxxxx>