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

[Nathan Chancellor <nathan@kernel.org>]

Hi Robertus,

On Thu, Jun 18, 2026 at 04:46:35AM +0700, Robertus Diawan Chris wrote:
> 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().

Yeah, I can definitely see why you went the route that you did.

> > 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.

Sounds good to me!

> > 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.

I do not mind taking patches that silence static checker warnings when
they are either small or do not make the code much more complicated to
understand. I think this change would satisfy both of those points, so
feel free to send a v2. I just do not see these type of fixes as high
priority.

-- 
Cheers,
Nathan