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

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

[Thorsten Leemhuis]

On 9/11/26 16:13, Masami Hiramatsu (Google) wrote:
> 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>

Masami, Andrew, this patch is now twice in -next:

e732d7c18d4d6a ("tools/bootconfig: consolidate xbc_init() to error
message wrapper") [merged via: "9eae8e94e61f20 ("Merge branch
'mm-nonmm-stable' of
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm")"; next-20261012]

78715511921cc8 ("tools/bootconfig: Consolidate xbc_init() to error
message wrapper") [merged via: "71ee85976d09e8 ("Merge branch 'for-next'
of
https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git")";
next-20261013]

Which I guess it what caused this build error for me:
""">> main.c:53:13: error: redefinition of ‘show_xbc_error’
>>    53 | static void show_xbc_error(const char *data, const char *msg, int pos)
>>       |             ^~~~~~~~~~~~~~
>> main.c:20:13: note: previous definition of ‘show_xbc_error’ with type ‘void(const char *, const char *, int)’
>>    20 | static void show_xbc_error(const char *data, const char *msg, int pos)
>>       |             ^~~~~~~~~~~~~~
>> main.c:74:12: error: redefinition of ‘init_xbc_with_error’
>>    74 | static int init_xbc_with_error(char *buf, int len)
>>       |            ^~~~~~~~~~~~~~~~~~~
>> main.c:41:12: note: previous definition of ‘init_xbc_with_error’ with type ‘int(char *, int)’
>>    41 | static int init_xbc_with_error(char *buf, int len)
>>       |            ^~~~~~~~~~~~~~~~~~~
>> main.c:41:12: warning: ‘init_xbc_with_error’ defined but not used [-Wunused-function]
>> make: *** [Makefile:21: bootconfig] Error 1
"""
Ciao, Thorsten

> ---
> 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);

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

[Andrew Morton]

On Wed, 14 Oct 2026 06:04:52 +0200 Thorsten Leemhuis <linux@leemhuis.info> wrote:

> On 9/11/26 16:13, Masami Hiramatsu (Google) wrote:
> > 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>
> 
> Masami, Andrew, this patch is now twice in -next:
> 
> e732d7c18d4d6a ("tools/bootconfig: consolidate xbc_init() to error
> message wrapper") [merged via: "9eae8e94e61f20 ("Merge branch
> 'mm-nonmm-stable' of
> https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm")"; next-20261012]
> 
> 78715511921cc8 ("tools/bootconfig: Consolidate xbc_init() to error
> message wrapper") [merged via: "71ee85976d09e8 ("Merge branch 'for-next'
> of
> https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git")";
> next-20261013]

OK, thanks, I'll drop the mm.git copy.

I was surprised to see bootconfig patches in the linux-trace tree, but
it seems that's where they're hosted.

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

[Steven Rostedt]

On Tue, 13 Oct 2026 21:23:51 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> I was surprised to see bootconfig patches in the linux-trace tree, but
> it seems that's where they're hosted.

That's because the bootconfig patches were originally created to add more
complex tracing command line options.

-- Steve

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

[Masami Hiramatsu]

On Tue, 13 Oct 2026 21:23:51 -0700, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 14 Oct 2026 06:04:52 +0200 Thorsten Leemhuis <linux@leemhuis.info> wrote:
>
> > On 9/11/26 16:13, Masami Hiramatsu (Google) wrote:
> > > 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>
> > 
> > Masami, Andrew, this patch is now twice in -next:
> > 
> > e732d7c18d4d6a ("tools/bootconfig: consolidate xbc_init() to error
> > message wrapper") [merged via: "9eae8e94e61f20 ("Merge branch
> > 'mm-nonmm-stable' of
> > https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm")"; next-20261012]
> > 
> > 78715511921cc8 ("tools/bootconfig: Consolidate xbc_init() to error
> > message wrapper") [merged via: "71ee85976d09e8 ("Merge branch 'for-next'
> > of
> > https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git")";
> > next-20261013]
>
> OK, thanks, I'll drop the mm.git copy.

Thanks Andrew, I'll keep this in bootconfig/for-next branch. 

>
> I was surprised to see bootconfig patches in the linux-trace tree, but
> it seems that's where they're hosted.

Yes, since it was introduced for boot-time tracing originally.

Thank you,

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