linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 2/4] Renames variable to fix shadow warning.
@ 2018-10-17  0:08 Leonardo Brás
  2018-10-17  2:56 ` [Lkcamp] " Helen Koike
  2018-10-17  6:01 ` Ingo Molnar
  0 siblings, 2 replies; 8+ messages in thread
From: Leonardo Brás @ 2018-10-17  0:08 UTC (permalink / raw)
  To: lkcamp
  Cc: Matthew Wilcox, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, x86, Masahiro Yamada,
	Michal Marek, linux-kernel, linux-kbuild

Renames the char variable to avoid shadowing a variable previously
declared on this function.

Signed-off-by: Leonardo Brás <leobras.c@gmail.com>
---
 arch/x86/entry/vdso/vdso2c.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/entry/vdso/vdso2c.h b/arch/x86/entry/vdso/vdso2c.h
index fa847a620f40..9466998d0f28 100644
--- a/arch/x86/entry/vdso/vdso2c.h
+++ b/arch/x86/entry/vdso/vdso2c.h
@@ -93,11 +93,11 @@ static void BITSFUNC(go)(void *raw_addr, size_t raw_len,
 		int k;
 		ELF(Sym) *sym = raw_addr + GET_LE(&symtab_hdr->sh_offset) +
 			GET_LE(&symtab_hdr->sh_entsize) * i;
-		const char *name = raw_addr + GET_LE(&strtab_hdr->sh_offset) +
+		const char *name2 = raw_addr + GET_LE(&strtab_hdr->sh_offset) +
 			GET_LE(&sym->st_name);
 
 		for (k = 0; k < NSYMS; k++) {
-			if (!strcmp(name, required_syms[k].name)) {
+			if (!strcmp(name2, required_syms[k].name)) {
 				if (syms[k]) {
 					fail("duplicate symbol %s\n",
 					     required_syms[k].name);
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Lkcamp] [PATCH 2/4] Renames variable to fix shadow warning.
  2018-10-17  0:08 [PATCH 2/4] Renames variable to fix shadow warning Leonardo Brás
@ 2018-10-17  2:56 ` Helen Koike
  2018-10-17 23:10   ` Leonardo Bras
  2018-10-17  6:01 ` Ingo Molnar
  1 sibling, 1 reply; 8+ messages in thread
From: Helen Koike @ 2018-10-17  2:56 UTC (permalink / raw)
  To: Leonardo Brás, lkcamp
  Cc: x86, linux-kbuild, Matthew Wilcox, linux-kernel, Masahiro Yamada,
	Ingo Molnar, Borislav Petkov, Andy Lutomirski, H. Peter Anvin,
	Michal Marek, Thomas Gleixner

Hi Leonardo,

Thanks for the patch, just some small comments below.

Please, check previous log messages with git log
arch/x86/entry/vdso/vdso2c.h, you will see that most patches had the
prefix "x86/vdso:" in the commit message.

On 10/16/18 9:08 PM, Leonardo Brás wrote:
> Renames the char variable to avoid shadowing a variable previously
> declared on this function.
> 
> Signed-off-by: Leonardo Brás <leobras.c@gmail.com>
> ---
>  arch/x86/entry/vdso/vdso2c.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/entry/vdso/vdso2c.h b/arch/x86/entry/vdso/vdso2c.h
> index fa847a620f40..9466998d0f28 100644
> --- a/arch/x86/entry/vdso/vdso2c.h
> +++ b/arch/x86/entry/vdso/vdso2c.h
> @@ -93,11 +93,11 @@ static void BITSFUNC(go)(void *raw_addr, size_t raw_len,
>  		int k;
>  		ELF(Sym) *sym = raw_addr + GET_LE(&symtab_hdr->sh_offset) +
>  			GET_LE(&symtab_hdr->sh_entsize) * i;
> -		const char *name = raw_addr + GET_LE(&strtab_hdr->sh_offset) +
> +		const char *name2 = raw_addr + GET_LE(&strtab_hdr->sh_offset) +
>  			GET_LE(&sym->st_name);

Could you please use a more meaningful name instead of name2 please? I
believe sym_name should be fine.

>  
>  		for (k = 0; k < NSYMS; k++) {
> -			if (!strcmp(name, required_syms[k].name)) {
> +			if (!strcmp(name2, required_syms[k].name)) {
>  				if (syms[k]) {
>  					fail("duplicate symbol %s\n",
>  					     required_syms[k].name);
> 

Regards
Helen

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/4] Renames variable to fix shadow warning.
  2018-10-17  0:08 [PATCH 2/4] Renames variable to fix shadow warning Leonardo Brás
  2018-10-17  2:56 ` [Lkcamp] " Helen Koike
@ 2018-10-17  6:01 ` Ingo Molnar
  2018-10-17 17:54   ` Andy Lutomirski
  2018-10-17 23:12   ` Leonardo Bras
  1 sibling, 2 replies; 8+ messages in thread
From: Ingo Molnar @ 2018-10-17  6:01 UTC (permalink / raw)
  To: Leonardo Brás
  Cc: lkcamp, Matthew Wilcox, Andy Lutomirski, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86,
	Masahiro Yamada, Michal Marek, linux-kernel, linux-kbuild


* Leonardo Brás <leobras.c@gmail.com> wrote:

> Renames the char variable to avoid shadowing a variable previously
> declared on this function.
> 
> Signed-off-by: Leonardo Brás <leobras.c@gmail.com>
> ---
>  arch/x86/entry/vdso/vdso2c.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/entry/vdso/vdso2c.h b/arch/x86/entry/vdso/vdso2c.h
> index fa847a620f40..9466998d0f28 100644
> --- a/arch/x86/entry/vdso/vdso2c.h
> +++ b/arch/x86/entry/vdso/vdso2c.h
> @@ -93,11 +93,11 @@ static void BITSFUNC(go)(void *raw_addr, size_t raw_len,
>  		int k;
>  		ELF(Sym) *sym = raw_addr + GET_LE(&symtab_hdr->sh_offset) +
>  			GET_LE(&symtab_hdr->sh_entsize) * i;
> -		const char *name = raw_addr + GET_LE(&strtab_hdr->sh_offset) +
> +		const char *name2 = raw_addr + GET_LE(&strtab_hdr->sh_offset) +
>  			GET_LE(&sym->st_name);
>  
>  		for (k = 0; k < NSYMS; k++) {
> -			if (!strcmp(name, required_syms[k].name)) {
> +			if (!strcmp(name2, required_syms[k].name)) {
>  				if (syms[k]) {
>  					fail("duplicate symbol %s\n",
>  					     required_syms[k].name);

NAK.

Please read and understand the code and rename both variables to 
meaningful names, not just a mindless name/name2 ...

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/4] Renames variable to fix shadow warning.
  2018-10-17  6:01 ` Ingo Molnar
@ 2018-10-17 17:54   ` Andy Lutomirski
  2018-10-17 23:16     ` Leonardo Bras
  2018-10-17 23:12   ` Leonardo Bras
  1 sibling, 1 reply; 8+ messages in thread
From: Andy Lutomirski @ 2018-10-17 17:54 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: leobras.c, lkcamp, Matthew Wilcox, Andrew Lutomirski,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	X86 ML, Masahiro Yamada, michal.lkml, LKML,
	Linux Kbuild mailing list

On Tue, Oct 16, 2018 at 11:01 PM Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Leonardo Brás <leobras.c@gmail.com> wrote:
>
> > Renames the char variable to avoid shadowing a variable previously
> > declared on this function.
> >
> > Signed-off-by: Leonardo Brás <leobras.c@gmail.com>
> > ---
> >  arch/x86/entry/vdso/vdso2c.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/entry/vdso/vdso2c.h b/arch/x86/entry/vdso/vdso2c.h
> > index fa847a620f40..9466998d0f28 100644
> > --- a/arch/x86/entry/vdso/vdso2c.h
> > +++ b/arch/x86/entry/vdso/vdso2c.h
> > @@ -93,11 +93,11 @@ static void BITSFUNC(go)(void *raw_addr, size_t raw_len,
> >               int k;
> >               ELF(Sym) *sym = raw_addr + GET_LE(&symtab_hdr->sh_offset) +
> >                       GET_LE(&symtab_hdr->sh_entsize) * i;
> > -             const char *name = raw_addr + GET_LE(&strtab_hdr->sh_offset) +
> > +             const char *name2 = raw_addr + GET_LE(&strtab_hdr->sh_offset) +
> >                       GET_LE(&sym->st_name);
> >
> >               for (k = 0; k < NSYMS; k++) {
> > -                     if (!strcmp(name, required_syms[k].name)) {
> > +                     if (!strcmp(name2, required_syms[k].name)) {
> >                               if (syms[k]) {
> >                                       fail("duplicate symbol %s\n",
> >                                            required_syms[k].name);
>
> NAK.
>
> Please read and understand the code and rename both variables to
> meaningful names, not just a mindless name/name2 ...
>

Maybe image_name and sym_name?

--Andy

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Lkcamp] [PATCH 2/4] Renames variable to fix shadow warning.
  2018-10-17  2:56 ` [Lkcamp] " Helen Koike
@ 2018-10-17 23:10   ` Leonardo Bras
  0 siblings, 0 replies; 8+ messages in thread
From: Leonardo Bras @ 2018-10-17 23:10 UTC (permalink / raw)
  To: helen
  Cc: lkcamp, x86, linux-kbuild, Matthew Wilcox, linux-kernel,
	Masahiro Yamada, Ingo Molnar, Borislav Petkov, Andy Lutomirski,
	H. Peter Anvin, Michal Marek, Thomas Gleixner

Hello Helen,
Thanks for the suggestions!

On Tue, Oct 16, 2018 at 11:57 PM Helen Koike <helen@koikeco.de> wrote:
>
> Hi Leonardo,
>
> Thanks for the patch, just some small comments below.
>
> Please, check previous log messages with git log
> arch/x86/entry/vdso/vdso2c.h, you will see that most patches had the
> prefix "x86/vdso:" in the commit message.
Ok, added! This change will be available on v2.

>
> On 10/16/18 9:08 PM, Leonardo Brás wrote:
> > Renames the char variable to avoid shadowing a variable previously
> > declared on this function.
> >
> > Signed-off-by: Leonardo Brás <leobras.c@gmail.com>
> > ---
> >  arch/x86/entry/vdso/vdso2c.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/entry/vdso/vdso2c.h b/arch/x86/entry/vdso/vdso2c.h
> > index fa847a620f40..9466998d0f28 100644
> > --- a/arch/x86/entry/vdso/vdso2c.h
> > +++ b/arch/x86/entry/vdso/vdso2c.h
> > @@ -93,11 +93,11 @@ static void BITSFUNC(go)(void *raw_addr, size_t raw_len,
> >               int k;
> >               ELF(Sym) *sym = raw_addr + GET_LE(&symtab_hdr->sh_offset) +
> >                       GET_LE(&symtab_hdr->sh_entsize) * i;
> > -             const char *name = raw_addr + GET_LE(&strtab_hdr->sh_offset) +
> > +             const char *name2 = raw_addr + GET_LE(&strtab_hdr->sh_offset) +
> >                       GET_LE(&sym->st_name);
>
> Could you please use a more meaningful name instead of name2 please? I
> believe sym_name should be fine.

Ok, I renamed as you suggested.
Also, I renamed the "name" variable to "image_name" as Andy suggested.
This change will be available on v2.

>
> >
> >               for (k = 0; k < NSYMS; k++) {
> > -                     if (!strcmp(name, required_syms[k].name)) {
> > +                     if (!strcmp(name2, required_syms[k].name)) {
> >                               if (syms[k]) {
> >                                       fail("duplicate symbol %s\n",
> >                                            required_syms[k].name);
> >
>
> Regards
> Helen

Regards,
Leonardo Bras

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/4] Renames variable to fix shadow warning.
  2018-10-17  6:01 ` Ingo Molnar
  2018-10-17 17:54   ` Andy Lutomirski
@ 2018-10-17 23:12   ` Leonardo Bras
  2018-10-18  5:47     ` Ingo Molnar
  1 sibling, 1 reply; 8+ messages in thread
From: Leonardo Bras @ 2018-10-17 23:12 UTC (permalink / raw)
  To: mingo
  Cc: lkcamp, Matthew Wilcox, Andy Lutomirski, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86,
	Masahiro Yamada, Michal Marek, linux-kernel, linux-kbuild

Thanks Ingo,
On Wed, Oct 17, 2018 at 3:01 AM Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Leonardo Brás <leobras.c@gmail.com> wrote:
>
> > Renames the char variable to avoid shadowing a variable previously
> > declared on this function.
> >
> > Signed-off-by: Leonardo Brás <leobras.c@gmail.com>
> > ---
> >  arch/x86/entry/vdso/vdso2c.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/entry/vdso/vdso2c.h b/arch/x86/entry/vdso/vdso2c.h
> > index fa847a620f40..9466998d0f28 100644
> > --- a/arch/x86/entry/vdso/vdso2c.h
> > +++ b/arch/x86/entry/vdso/vdso2c.h
> > @@ -93,11 +93,11 @@ static void BITSFUNC(go)(void *raw_addr, size_t raw_len,
> >               int k;
> >               ELF(Sym) *sym = raw_addr + GET_LE(&symtab_hdr->sh_offset) +
> >                       GET_LE(&symtab_hdr->sh_entsize) * i;
> > -             const char *name = raw_addr + GET_LE(&strtab_hdr->sh_offset) +
> > +             const char *name2 = raw_addr + GET_LE(&strtab_hdr->sh_offset) +
> >                       GET_LE(&sym->st_name);
> >
> >               for (k = 0; k < NSYMS; k++) {
> > -                     if (!strcmp(name, required_syms[k].name)) {
> > +                     if (!strcmp(name2, required_syms[k].name)) {
> >                               if (syms[k]) {
> >                                       fail("duplicate symbol %s\n",
> >                                            required_syms[k].name);
>
> NAK.
>
> Please read and understand the code and rename both variables to
> meaningful names, not just a mindless name/name2 ...
>

It's changed! This change will be available on v2.

> Thanks,
>
>         Ingo

Regards,
Leonardo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/4] Renames variable to fix shadow warning.
  2018-10-17 17:54   ` Andy Lutomirski
@ 2018-10-17 23:16     ` Leonardo Bras
  0 siblings, 0 replies; 8+ messages in thread
From: Leonardo Bras @ 2018-10-17 23:16 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: mingo, lkcamp, Matthew Wilcox, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, x86, Masahiro Yamada,
	Michal Marek, linux-kernel, linux-kbuild

Hello Andy,
Thanks for the suggestion.
I renamed them as suggested and it will be available on v2.

Regards,

Leonardo

On Wed, Oct 17, 2018 at 2:54 PM Andy Lutomirski <luto@kernel.org> wrote:
>
> On Tue, Oct 16, 2018 at 11:01 PM Ingo Molnar <mingo@kernel.org> wrote:
> >
> >
> > * Leonardo Brás <leobras.c@gmail.com> wrote:
> >
> > > Renames the char variable to avoid shadowing a variable previously
> > > declared on this function.
> > >
> > > Signed-off-by: Leonardo Brás <leobras.c@gmail.com>
> > > ---
> > >  arch/x86/entry/vdso/vdso2c.h | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/x86/entry/vdso/vdso2c.h b/arch/x86/entry/vdso/vdso2c.h
> > > index fa847a620f40..9466998d0f28 100644
> > > --- a/arch/x86/entry/vdso/vdso2c.h
> > > +++ b/arch/x86/entry/vdso/vdso2c.h
> > > @@ -93,11 +93,11 @@ static void BITSFUNC(go)(void *raw_addr, size_t raw_len,
> > >               int k;
> > >               ELF(Sym) *sym = raw_addr + GET_LE(&symtab_hdr->sh_offset) +
> > >                       GET_LE(&symtab_hdr->sh_entsize) * i;
> > > -             const char *name = raw_addr + GET_LE(&strtab_hdr->sh_offset) +
> > > +             const char *name2 = raw_addr + GET_LE(&strtab_hdr->sh_offset) +
> > >                       GET_LE(&sym->st_name);
> > >
> > >               for (k = 0; k < NSYMS; k++) {
> > > -                     if (!strcmp(name, required_syms[k].name)) {
> > > +                     if (!strcmp(name2, required_syms[k].name)) {
> > >                               if (syms[k]) {
> > >                                       fail("duplicate symbol %s\n",
> > >                                            required_syms[k].name);
> >
> > NAK.
> >
> > Please read and understand the code and rename both variables to
> > meaningful names, not just a mindless name/name2 ...
> >
>
> Maybe image_name and sym_name?
>
> --Andy

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/4] Renames variable to fix shadow warning.
  2018-10-17 23:12   ` Leonardo Bras
@ 2018-10-18  5:47     ` Ingo Molnar
  0 siblings, 0 replies; 8+ messages in thread
From: Ingo Molnar @ 2018-10-18  5:47 UTC (permalink / raw)
  To: Leonardo Bras
  Cc: lkcamp, Matthew Wilcox, Andy Lutomirski, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86,
	Masahiro Yamada, Michal Marek, linux-kernel, linux-kbuild


* Leonardo Bras <leobras.c@gmail.com> wrote:

> Thanks Ingo,
> On Wed, Oct 17, 2018 at 3:01 AM Ingo Molnar <mingo@kernel.org> wrote:
> >
> >
> > * Leonardo Brás <leobras.c@gmail.com> wrote:
> >
> > > Renames the char variable to avoid shadowing a variable previously
> > > declared on this function.
> > >
> > > Signed-off-by: Leonardo Brás <leobras.c@gmail.com>
> > > ---
> > >  arch/x86/entry/vdso/vdso2c.h | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/x86/entry/vdso/vdso2c.h b/arch/x86/entry/vdso/vdso2c.h
> > > index fa847a620f40..9466998d0f28 100644
> > > --- a/arch/x86/entry/vdso/vdso2c.h
> > > +++ b/arch/x86/entry/vdso/vdso2c.h
> > > @@ -93,11 +93,11 @@ static void BITSFUNC(go)(void *raw_addr, size_t raw_len,
> > >               int k;
> > >               ELF(Sym) *sym = raw_addr + GET_LE(&symtab_hdr->sh_offset) +
> > >                       GET_LE(&symtab_hdr->sh_entsize) * i;
> > > -             const char *name = raw_addr + GET_LE(&strtab_hdr->sh_offset) +
> > > +             const char *name2 = raw_addr + GET_LE(&strtab_hdr->sh_offset) +
> > >                       GET_LE(&sym->st_name);
> > >
> > >               for (k = 0; k < NSYMS; k++) {
> > > -                     if (!strcmp(name, required_syms[k].name)) {
> > > +                     if (!strcmp(name2, required_syms[k].name)) {
> > >                               if (syms[k]) {
> > >                                       fail("duplicate symbol %s\n",
> > >                                            required_syms[k].name);
> >
> > NAK.
> >
> > Please read and understand the code and rename both variables to
> > meaningful names, not just a mindless name/name2 ...
> >
> 
> It's changed! This change will be available on v2.

Thanks!

	Ingo

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-10-18  5:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-17  0:08 [PATCH 2/4] Renames variable to fix shadow warning Leonardo Brás
2018-10-17  2:56 ` [Lkcamp] " Helen Koike
2018-10-17 23:10   ` Leonardo Bras
2018-10-17  6:01 ` Ingo Molnar
2018-10-17 17:54   ` Andy Lutomirski
2018-10-17 23:16     ` Leonardo Bras
2018-10-17 23:12   ` Leonardo Bras
2018-10-18  5:47     ` Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).