Re: [PATCH] fs: binfmt_elf_efpic: fix variable set but not used warning

[Sunliming]

On 2025/3/8 04:30, Kees Cook wrote:
> On Fri, Mar 07, 2025 at 03:47:28PM +0100, Jan Kara wrote:
> > On Fri 07-03-25 14:11:28, sunliming@xxxxxxxxx wrote:
> > > From: sunliming <sunliming@xxxxxxxxxx>
> > >
> > > Fix below kernel warning:
> > > fs/binfmt_elf_fdpic.c:1024:52: warning: variable 'excess1' set but not
> > > used [-Wunused-but-set-variable]
> > >
> > > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > > Signed-off-by: sunliming <sunliming@xxxxxxxxxx>
> > The extra ifdef is not pretty but I guess it's better. Feel free to add:
> >
> > Reviewed-by: Jan Kara <jack@xxxxxxx>
> Since we allow loop-scope variable definitions now, perhaps this is a
> case for defining the variable later within the #ifdef, like this?
>
>
> diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
> index e3cf2801cd64..b0ef71238328 100644
> --- a/fs/binfmt_elf_fdpic.c
> +++ b/fs/binfmt_elf_fdpic.c
> @@ -1024,7 +1024,7 @@ static int elf_fdpic_map_file_by_direct_mmap(struct elf_fdpic_params *params,
>   	/* deal with each load segment separately */
>   	phdr = params->phdrs;
>   	for (loop = 0; loop < params->hdr.e_phnum; loop++, phdr++) {
> -		unsigned long maddr, disp, excess, excess1;
> +		unsigned long maddr, disp, excess;
>   		int prot = 0, flags;
>   
>   		if (phdr->p_type != PT_LOAD)
> @@ -1120,9 +1120,10 @@ static int elf_fdpic_map_file_by_direct_mmap(struct elf_fdpic_params *params,
>   		 *   extant in the file
>   		 */
>   		excess = phdr->p_memsz - phdr->p_filesz;
> -		excess1 = PAGE_SIZE - ((maddr + phdr->p_filesz) & ~PAGE_MASK);
>   
>   #ifdef CONFIG_MMU
> +		const unsigned long excess1 =
> +			PAGE_SIZE - ((maddr + phdr->p_filesz) & ~PAGE_MASK);
>   		if (excess > excess1) {
>   			unsigned long xaddr = maddr + phdr->p_filesz + excess1;
>   			unsigned long xmaddr;
I think this is a good idea. I will resend this patch in this way，thanks.
> > 								Honza
> >
> > > ---
> > >   fs/binfmt_elf_fdpic.c | 7 +++++--
> > >   1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
> > > index e3cf2801cd64..bed13ee8bfec 100644
> > > --- a/fs/binfmt_elf_fdpic.c
> > > +++ b/fs/binfmt_elf_fdpic.c
> > > @@ -1024,8 +1024,11 @@ static int elf_fdpic_map_file_by_direct_mmap(struct elf_fdpic_params *params,
> > >   	/* deal with each load segment separately */
> > >   	phdr = params->phdrs;
> > >   	for (loop = 0; loop < params->hdr.e_phnum; loop++, phdr++) {
> > > -		unsigned long maddr, disp, excess, excess1;
> > > +		unsigned long maddr, disp, excess;
> > >   		int prot = 0, flags;
> > > +#ifdef CONFIG_MMU
> > > +		unsigned long excess1;
> > > +#endif
> > >   
> > >   		if (phdr->p_type != PT_LOAD)
> > >   			continue;
> > > @@ -1120,9 +1123,9 @@ static int elf_fdpic_map_file_by_direct_mmap(struct elf_fdpic_params *params,
> > >   		 *   extant in the file
> > >   		 */
> > >   		excess = phdr->p_memsz - phdr->p_filesz;
> > > -		excess1 = PAGE_SIZE - ((maddr + phdr->p_filesz) & ~PAGE_MASK);
> > >   
> > >   #ifdef CONFIG_MMU
> > > +		excess1 = PAGE_SIZE - ((maddr + phdr->p_filesz) & ~PAGE_MASK);
> > >   		if (excess > excess1) {
> > >   			unsigned long xaddr = maddr + phdr->p_filesz + excess1;
> > >   			unsigned long xmaddr;
> > > --
> > > 2.25.1
> > --
> > Jan Kara <jack@xxxxxxxx>
> > SUSE Labs, CR