Re: [PATCH] perf tools: Remove redundant variable assignment

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Mon, Nov 11, 2024 at 02:42:46PM -0300, Arnaldo Carvalho de Melo wrote:
> On Mon, Nov 11, 2024 at 04:27:13PM +0800, Luo Yifan wrote:
> > This patch makes a minor change that removes the redundant assignment
> > to the variable ret, simplifying the code.
> 
> Thanks, applied to perf-tools-next,

Are you build testing these patches?

 GEN     perf-archive
  CC      /tmp/build/perf-tools-next/libsubcmd/sigchain.o
  GEN     perf-iostat
  INSTALL libbpf_headers
  LD      /tmp/build/perf-tools-next/libsubcmd/libsubcmd-in.o
  AR      /tmp/build/perf-tools-next/libsubcmd/libsubcmd.a
jvmti/jvmti_agent.c: In function ‘jvmti_write_code’:
jvmti/jvmti_agent.c:366:13: error: variable ‘ret’ set but not used [-Werror=unused-but-set-variable]
  366 |         int ret = -1;
      |             ^~~
cc1: all warnings being treated as errors
make[3]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:106: /tmp/build/perf-tools-next/jvmti/jvmti_agent.o] Error 1
make[2]: *** [Makefile.perf:936: /tmp/build/perf-tools-next/jvmti/jvmti-in.o] Error 2
make[2]: *** Waiting for unfinished jobs....
  CC      /tmp/build/perf-tools-next/util/header.o
  LD      /tmp/build/perf-tools-next/util/perf-util-in.o
  LD      /tmp/build/perf-tools-next/perf-util-in.o
make[1]: *** [Makefile.perf:292: sub-make] Error 2
make: *** [Makefile:119: install-bin] Error 2
make: Leaving directory '/home/acme/git/perf-tools-next/tools/perf'
⬢ [acme@toolbox perf-tools-next]$

The original patch by Stephane has it right, that initial ret = -1 is
used when there are other problems and the code goes to a label at the
end, returning that -1.

But the code was changed later and problems were introduced, so you
removed something simple at the end and somehow missed that it breaks
the build (at least for me) and when I go look at the code, I see the
other problems, so please take the time to try and investigate this and
fix the 'ret' variable usage.

I'm removing this patch from my local tree.

Thanks,
 
> - Arnaldo
>  
> > Signed-off-by: Luo Yifan <luoyifan@cmss.chinamobile.com>
> > ---
> >  tools/perf/jvmti/jvmti_agent.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/tools/perf/jvmti/jvmti_agent.c b/tools/perf/jvmti/jvmti_agent.c
> > index 526dcaf9f..751219143 100644
> > --- a/tools/perf/jvmti/jvmti_agent.c
> > +++ b/tools/perf/jvmti/jvmti_agent.c
> > @@ -408,9 +408,7 @@ jvmti_write_code(void *agent, char const *sym,
> >  
> >  	funlockfile(fp);
> >  
> > -	ret = 0;
> > -
> > -	return ret;
> > +	return 0;
> >  }
> >  
> >  int
> > -- 
> > 2.27.0
> > 
> >