Re: [PATCH 2/2] Use any previous initializer to size a symbol

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Mar 30, 2014 at 10:22 AM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> This seems to be the simplest approach. Chris, what was your concern about
> scoping? The "same_symbol" list should already take scoping into account.

Ho humm. I found a test-case that still messes up:

  extern const char *v4l2_type_names[];
  const char *v4l2_type_names[] = {
    "test", "hello"
  };

  static unsigned test(void)
  {
    extern const char *v4l2_type_names[];
    return sizeof(v4l2_type_names);
  }

and the reason seems to be that the 'same_symbol' list is incomplete.

What happen sis that we tie the two "extern" v4l2_type_names[] things
together, and the declaration of v4l2_type_names[] with the
initializer is also tied to the first 'extern' declaration, but we do
*not* tie the second 'extern' in block scope to the one with the
initializer.

The reason seems to be that our logic in check_declaration[] is
broken: it only ties things together based on both symbols having
'extern'. So the actual declaration with an initializer, that lacks
the extern (but is in global scope) is missed.

The attached patch would seem to fix it. Chris, what is the reason for
that MOD_INLINE check? You added it in commit 8cf99394ee4c ("move
extern inline function to file scope") and I suspect it messes up the
same_symbol logic. But I left it alone, and added a comment. If you
can resolve that comment, please apply this patch with my sign-off and
some random commit message..

                      Linus
 symbol.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/symbol.c b/symbol.c
index 4b91abd8021e..c5e4784734a8 100644
--- a/symbol.c
+++ b/symbol.c
@@ -576,11 +576,15 @@ void check_declaration(struct symbol *sym)
 			sym->same_symbol = next;
 			return;
 		}
-		if (sym->ctype.modifiers & next->ctype.modifiers & MOD_EXTERN) {
-			if ((sym->ctype.modifiers ^ next->ctype.modifiers) & MOD_INLINE)
-				continue;
-			sym->same_symbol = next;
-			return;
+		/* Extern in block level matches a TOPLEVEL non-static symbol */
+		if (sym->ctype.modifiers & MOD_EXTERN) {
+			if ((next->ctype.modifiers & (MOD_TOPLEVEL|MOD_STATIC)) == MOD_TOPLEVEL) {
+				/* FIXME? Differs in 'inline' only? Why does that matter? */
+				if ((sym->ctype.modifiers ^ next->ctype.modifiers) & MOD_INLINE)
+					continue;
+				sym->same_symbol = next;
+				return;
+			}
 		}
 
 		if (!Wshadow || warned)

[Index of Archives]     [Newbies FAQ]     [LKML]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Trinity Fuzzer Tool]

  Powered by Linux