Re: [PATCH] modpost: release allocation when early return no suffix .o in read_symbols()

Robertus Diawan Chris <[email protected]>
Newsgroups org.kernel.vger.linux-kbuild,dev.linux.lists.linux-kernel-mentees,org.kernel.vger.linux-kernel
Message-ID <20260617214635.GA4766@soyboi>
Hello Nathan,

On Tue, Jun 16, 2026 at 09:21:52PM -0700, Nathan Chancellor wrote:
> On Wed, 10 Jun 2026 12:25:50 +0700, Robertus Diawan Chris <[email protected]> wrote:
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index abbcd3fc1394..8e231544f9f3 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -1585,6 +1585,7 @@ static void read_symbols(const char *modname)
> >
> >  	if (!strends(modname, ".o")) {
> >  		error("%s: filename must be suffixed with .o\n", modname);
> > +		parse_elf_finish(&info);
> >  		return;
> >  	}
> >
>
> Thanks for the patch! While this change appears to be correct, would
> moving the strends() if block before the parse_elf() one resolve this as
> well?

Yes, moving the strends() if block before the parse_elf() also resolve
this too. The reason I didn't do that because I am still not sure if
changing the order will have any effect. I already take a look at the
code and it looks like strends() if block and the parse_elf() didn't
depends on each other, but just in case I missed something, I decided
not to change the execution order and just add parse_elf_finish().

> I think I would prefer going that route because neither check really
> depends on the other and we have to think less about unwinding
> with the checks flipped.

Now that you already confirm that neither check depends on each other, I
am more confident to take this approach.

> Furthermore, modpost is a relatively short running host utility, so
> I don't really think it is worth optimizing for resource leaks like
> this.

I want to confirm something first, do you mind if I send v2 patch with
the change of moving strends() if block before parse_elf() in
read_symbols()? I mean, if you feel like this is unnecessary, I will
drop this patch. I don't mind either way.

Thank you for the feedback.

Best regards,
Robertus Diawan Chris
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.