* [PATCH] efi: Resolve some shadow warnings
@ 2014-09-06 1:34 Jeff Kirsher
2014-09-06 9:03 ` Matt Fleming
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Kirsher @ 2014-09-06 1:34 UTC (permalink / raw)
To: matt.fleming; +Cc: Mark Rustad, linux-efi, linux-kernel, 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] efi: Resolve some shadow warnings
2014-09-06 1:34 [PATCH] efi: Resolve some shadow warnings Jeff Kirsher
@ 2014-09-06 9:03 ` Matt Fleming
2014-09-06 9:15 ` Jeff Kirsher
0 siblings, 1 reply; 3+ messages in thread
From: Matt Fleming @ 2014-09-06 9:03 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: matt.fleming, Mark Rustad, linux-efi, linux-kernel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] efi: Resolve some shadow warnings
2014-09-06 9:03 ` Matt Fleming
@ 2014-09-06 9:15 ` Jeff Kirsher
0 siblings, 0 replies; 3+ messages in thread
From: Jeff Kirsher @ 2014-09-06 9:15 UTC (permalink / raw)
To: Matt Fleming; +Cc: matt.fleming, Mark Rustad, linux-efi, linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-09-06 9:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-06 1:34 [PATCH] efi: Resolve some shadow warnings Jeff Kirsher
2014-09-06 9:03 ` Matt Fleming
2014-09-06 9:15 ` Jeff Kirsher
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®