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

[Sang-Heon Jeon]

On Thu, Sep 10, 2026 at 12:53 AM 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>
> ---
>  tools/bootconfig/main.c |  108 ++++++++++++++++++++++-------------------------
>  1 file changed, 51 insertions(+), 57 deletions(-)
>
> diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c
> index 6035404733c3..7117aa9b2a83 100644
> --- a/tools/bootconfig/main.c
> +++ b/tools/bootconfig/main.c
> @@ -21,6 +21,47 @@
>  #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)
> +{
> +       char *copy = malloc(len);
> +       const char *msg;
> +       int ret, pos;
> +
> +       if (!copy)
> +               return -ENOMEM;
> +
> +       memcpy(copy, buf, len);

Can we remove this? xbc_init() already copies buf and does not change
it, so show_xbc_error() can use buf directly.


> +       /* We do not terminate the copy with \0 for sanity checking */
> +
> +       ret = xbc_init(buf, len, &msg, &pos);
> +       if (ret < 0)
> +               show_xbc_error(copy, msg, pos);
> +       free(copy);
> +
> +       return ret;
> +}
> +
>  static int xbc_show_value(struct xbc_node *node, bool semicolon)
>  {
>         const char *val, *eol;
> @@ -187,7 +228,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)
> @@ -238,52 +278,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;
> -       }
> +       ret = init_xbc_with_error(*buf, size);
>
> -       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);
> -
> -}
> -
> -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)
> @@ -412,9 +409,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);
> @@ -427,6 +423,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) {
> @@ -435,15 +438,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);
>