On Tue, Apr 1, 2025 at 10:34 PM Thomas Monjalon <tho...@monjalon.net> wrote:
>
> 28/03/2025 11:52, David Marchand:
> > +scriptname, link_mode, abi_version_file, output, *files = sys.argv
>
> You should add a comment to show the usage of the script.
>
> [...]
> > +if link_mode == 'mslinker':
> > + with open(output, "w") as outfile:
> > + print(f"EXPORTS", file=outfile)
> > + for key in (abi, 'EXPERIMENTAL', 'INTERNAL'):
> > + if key not in symbols:
> > + continue
> > + for symbol in sorted(symbols[key].keys()):
> > + print(f"\t{symbol}", file=outfile)
> > + del symbols[key]
> > +else:
>
> Maybe add a comment after the else to make explicit it is GNU format.
>
> [...]
> > +for file in sys.argv[1:]:
>
> This script requires a comment with the usage as well
> to make clear what we expect as arguments.
Ack.
>
> > + with open(file, encoding="utf-8") as f:
> > + for ln in f.readlines():
> > + if file_header_regexp.match(ln):
> > + if file_header_regexp.match(ln).group(2) == "lib":
> > + lib = '/'.join(file_header_regexp.match(ln).group(2,
> > 3))
> > + elif file_header_regexp.match(ln).group(3) == "intel":
> > + lib = '/'.join(file_header_regexp.match(ln).group(2,
> > 3, 4))
> > + else:
> > + lib = '/'.join(file_header_regexp.match(ln).group(2,
> > 3))
>
> [...]
> > +#define RTE_EXPORT_EXPERIMENTAL_SYMBOL(a, ver)
> > +#define RTE_EXPORT_INTERNAL_SYMBOL(a)
> > +#define RTE_EXPORT_SYMBOL(a)
>
> I would prefer an explicit variable name instead of "a".
Ack.
Thanks for the review.
--
David Marchand
