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

[Robertus Diawan Chris <robertusdchris@gmail.com>]

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 <robertusdchris@gmail.com> 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