lkml.org 
[lkml]   [2021]   [Oct]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] fortify: Avoid shadowing previous locals
On Mon, Oct 25, 2021 at 04:15:28PM -0400, Qian Cai wrote:
>
>
> On 10/25/21 3:34 PM, Kees Cook wrote:
> > On Mon, Oct 25, 2021 at 02:37:28PM -0400, Qian Cai wrote:
> >> __compiletime_strlen macro expansion will shadow p_size and p_len local
> >> variables. Just rename those in __compiletime_strlen.
> >
> > They don't escape their local context, though, right? i.e. I don't see a
> > problem with the existing macro. Did you encounter a specific issue that
> > this patch fixes?
>
> Yes, this is pretty minor. There are also some extra compiling warnings (W=2)
> from it.
>
> ./include/linux/fortify-string.h: In function 'strnlen':
>
> ./include/linux/fortify-string.h:17:9: warning: declaration of 'p_size' shadows a previous local [-Wshadow]
>
> 17 | size_t p_size = __builtin_object_size(p, 1); \
>
> | ^~~~~~
>
> ./include/linux/fortify-string.h:77:17: note: in expansion of macro '__compiletime_strlen'
> 77 | size_t p_len = __compiletime_strlen(p);
>
> | ^~~~~~~~~~~~~~~~~~~~
>
> ./include/linux/fortify-string.h:76:9: note: shadowed declaration is here
>
> 76 | size_t p_size = __builtin_object_size(p, 1);
>
> | ^~~~~~

Gotcha. Yeah, we have -Wshadow=local tracked here:
https://github.com/KSPP/linux/issues/152

The changes needed to fix __wait_event() are extensive, though. But yes,
there's no good reason for this macro to make things worse for W=2. ;)

I'd like to keep the existing names, so many just prefixing them with
"__" and send a v2?

Thanks!

--
Kees Cook

\
 
 \ /
  Last update: 2021-10-25 22:50    [W:0.046 / U:0.336 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site