Re: [PATCH v3 next 02/13] scripts/kallsyms: Improve error reporting

[David Laight <david.laight.linux@gmail.com>]

On Wed, 11 Nov 2026 23:33:55 +0000 (UTC)
bot+bpf-ci@kernel.org wrote:

> > scripts/kallsyms: Improve error reporting  
> 
> TL;DR:
> - Error message for failing to open the output file says "nin.out", likely a
>   typo for "out.bin"

Indeed - will fix (and in #4)

> - usage() text for --all-symbols may be inaccurate: it covers all sections,
>   not just local symbols

It'll do - needs to be terse and it's a build tool.

David

> 
> > diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> > --- a/scripts/kallsyms.c
> > +++ b/scripts/kallsyms.c  
> 
> [ ... ]
> 
> > @@ -822,6 +824,25 @@ static void sort_symbols(void)
> >  	qsort(table, table_cnt, sizeof(table[0]), compare_symbols);
> >  }
> >  
> > +static void usage(void)
> > +{
> > +	fprintf(stderr,
> > +		"Usage: kallsyms [options] vmlinux out.bin > out.S\n"
> > +		"       kallsyms --sysmap=out.map vmlinux\n"
> > +		"           --all-symbols     Include all local symbols\n"  
> 
> This isn't a bug, but is this description of --all-symbols accurate?
> 
> In symbol_valid() the option only disables the text range filter:
> 
>     if (!all_symbols) {
>         ...
>         if (symbol_in_range(s, text_ranges,
>                             ARRAY_SIZE(text_ranges)) == 0)
>             return 0;
> 
> so symbols from the data, rodata and bss sections are included, global and
> local alike.  Local text symbols are already kept without the option.  This
> also matches the CONFIG_KALLSYMS_ALL help text, "symbols from all sections".
> 
> Would something like "Include symbols from all sections, not only text" be
> closer to what the code does?
> 
> [ ... ]
> 
> > @@ -845,49 +867,57 @@ int main(int argc, char **argv)  
> [ ... ]
> >  	out_bin_name = argv[optind + 1];
> >  	out_bin_file = fopen(out_bin_name, "w");
> > -	if (!out_bin_file) {
> > -		perror(out_bin_name);
> > -		exit(EXIT_FAILURE);
> > -	}
> > +	if (!out_bin_file)
> > +		err(EXIT_FAILURE, "failed to open nin.out file %s", out_bin_name);  
> 
> Is "nin.out" a typo here?  usage() and the close-time message in main(),
> 
>     errx(EXIT_FAILURE, "error writing out.bin file %s", out_bin_name);
> 
> both call this file "out.bin".
> 
> The string is changed later in the series by 317dc01f3774 ("scripts/kallsyms:
> make use of binary data output optional") to "failed to open bin.out file
> %s", which still differs from the "out.bin" used by usage() and the
> close-time message.
> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/43116767394