[PATCH] efi: Resolve some shadow warnings

[PATCH] efi: Resolve some shadow warnings

[Jeff Kirsher]

From: Mark Rustad <mark.d.rustad@intel.com>

It is a really bad idea to declare variables or parameters that
have the same name as common types. It is valid C, but it gets
surprising if a macro expansion attempts to declare an inner
local with that type. Change the local names to eliminate the
hazard.

Change s16 => ps16, s8 => ps8.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/firmware/efi/vars.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
index 5abe943..afa3596 100644
--- a/drivers/firmware/efi/vars.c
+++ b/drivers/firmware/efi/vars.c
@@ -321,11 +321,11 @@ static unsigned long var_name_strnsize(efi_char16_t *variable_name,
  * Print a warning when duplicate EFI variables are encountered and
  * disable the sysfs workqueue since the firmware is buggy.
  */
-static void dup_variable_bug(efi_char16_t *s16, efi_guid_t *vendor_guid,
+static void dup_variable_bug(efi_char16_t *ps16, efi_guid_t *vendor_guid,
 			     unsigned long len16)
 {
 	size_t i, len8 = len16 / sizeof(efi_char16_t);
-	char *s8;
+	char *ps8;
 
 	/*
 	 * Disable the workqueue since the algorithm it uses for
@@ -334,16 +334,16 @@ static void dup_variable_bug(efi_char16_t *s16, efi_guid_t *vendor_guid,
 	 */
 	efivar_wq_enabled = false;
 
-	s8 = kzalloc(len8, GFP_KERNEL);
-	if (!s8)
+	ps8 = kzalloc(len8, GFP_KERNEL);
+	if (!ps8)
 		return;
 
 	for (i = 0; i < len8; i++)
-		s8[i] = s16[i];
+		ps8[i] = ps16[i];
 
 	printk(KERN_WARNING "efivars: duplicate variable: %s-%pUl\n",
-	       s8, vendor_guid);
-	kfree(s8);
+	       ps8, vendor_guid);
+	kfree(ps8);
 }
 
 /**
-- 
1.9.3

Re: [PATCH] efi: Resolve some shadow warnings

[Matt Fleming]

On Fri, 05 Sep, at 06:34:52PM, Jeff Kirsher wrote:
> From: Mark Rustad <mark.d.rustad@intel.com>
> 
> It is a really bad idea to declare variables or parameters that
> have the same name as common types. It is valid C, but it gets
> surprising if a macro expansion attempts to declare an inner
> local with that type. Change the local names to eliminate the
> hazard.
 
Oops, good catch. Could you include a copy of the compiler shadow
warnings in the commit log? I'm guessing this bug is highlighted with
-Wshadow? How did you discover this problem?

> Change s16 => ps16, s8 => ps8.
 
Hmm... Instead, could you change it to str16 and str8? s8/s16 were
clearly misguided names in the first place, and it's not immediatealy
obvious to me that a 'ps16' would represent "pointer to 16-bit string".

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH] efi: Resolve some shadow warnings

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1255 bytes --]

On Sat, 2014-09-06 at 10:03 +0100, Matt Fleming wrote:
> On Fri, 05 Sep, at 06:34:52PM, Jeff Kirsher wrote:
> > From: Mark Rustad <mark.d.rustad@intel.com>
> > 
> > It is a really bad idea to declare variables or parameters that
> > have the same name as common types. It is valid C, but it gets
> > surprising if a macro expansion attempts to declare an inner
> > local with that type. Change the local names to eliminate the
> > hazard.
>  
> Oops, good catch. Could you include a copy of the compiler shadow
> warnings in the commit log? I'm guessing this bug is highlighted with
> -Wshadow? How did you discover this problem?

Mark was using W=2 to see the warning, so I will see if I can wade
through the numerous warnings that get generated with W=2 for the
specific warning.

> 
> > Change s16 => ps16, s8 => ps8.
>  
> Hmm... Instead, could you change it to str16 and str8? s8/s16 were
> clearly misguided names in the first place, and it's not immediatealy
> obvious to me that a 'ps16' would represent "pointer to 16-bit string".
> 

Ok, I can re-spin the patch for Mark, that is if I can capture the
original warning to add to his commit message.  Otherwise, I will wait
till Monday to have Mark re-spin the patch.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]