[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] maint.mk: add a syntax-check rule to ensure tightly-scoped s
From: |
Pádraig Brady |
Subject: |
Re: [PATCH] maint.mk: add a syntax-check rule to ensure tightly-scoped symbols |
Date: |
Tue, 17 May 2011 15:23:42 +0100 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3 |
On 17/05/11 15:08, Jim Meyering wrote:
> Pádraig Brady wrote:
>> On 09/05/11 11:44, Jim Meyering wrote:
>>> I've been using a precursor of this rule in coreutils for many years,
>>
>>> so finally have made it general enough so that the same
>>
>> The above change is giving a false positive on my 32 linux laptop,
>> where it flags __i686.get_pc_thunk.bx from src/libstdbuf_so-libstdbuf.o
>> The following fix auto excludes any symbols starting with an underscore.
>> Maybe I should do this only for double underscores?
>>
>> pb-laptop:~/git/coreutils$ diff gnulib/top/maint.mk maint.mk
>> --- gnulib/top/maint.mk 2011-05-14 09:30:58.000000000 +0000
>> +++ maint.mk 2011-05-17 12:45:56.000000000 +0000
>> @@ -1397,7 +1397,7 @@
>> perl -lne '$(_gl_TS_function_match)' \
>> -e 'and print $$1' $$hdr; \
>> ) | sort -u | sed 's/^/^/;s/$$/$$/' > $$t; \
>> - nm -e *.$(OBJEXT) | sed -n 's/.* T //p' | grep -Ev -f $$t \
>> + nm -e *.$(OBJEXT) | sed -n 's/.* T \([^_]\)/\1/p' | grep -Ev -f $$t \
>> && { echo the above functions should have static scope >&2; \
>> exit 1; } || : ; \
>> ( printf '^%s$$\n' $(_gl_TS_unmarked_extern_vars); \
>
> Hi Pádraig,
> I did hesitate as I removed the filter that ignored
> any name with a leading underscore. But since I didn't
> see any such symbol actually being removed in testing
> I chose to omit the filter.
>
> I'm reluctant to ignore all of them... what if someone accidentally
> declares e.g., int _foo () { ... }
> (omitting the "static")? I wouldn't want to fail to warn about that,
> just because their function name starts with "_".
>
> Two alternatives:
>
> - rather than hard-coding *.$(OBJEXT), use something like this:
> _gl_TS_function_objects ?= *.$(OBJEXT)
> and override it in coreutils to $(filter-out ...) that one
> offending .o file.
A bit hacky, and exludes that whole module from the check.
>
> - in coreutils, add __i686.get_pc_thunk.bx or maybe just '^__i686.*$$'
> to the list of _gl_TS_unmarked_extern_functions
A bit specific/fragile.
Given '^__.*' names are reserved by the compiler,
perhaps the happy medium is to to allow single underscores,
but exclude double underscores, with something like:
--- gnulib/top/maint.mk 2011-05-14 09:30:58.000000000 +0000
+++ maint.mk 2011-05-17 14:25:15.000000000 +0000
@@ -1362,14 +1362,15 @@
# Most functions should have static scope.
# Any that don't must be marked with `extern', but `main'
# and `usage' are exceptions: they're always extern, but
-# do not need to be marked.
-_gl_TS_unmarked_extern_functions ?= main usage
+# do not need to be marked. Symbols starting with `__'
+# are reserved by the compiler, so exclude those also.
+_gl_TS_unmarked_extern_functions ?= main usage __.*
_gl_TS_function_match ?= \
/^(?:extern|XTERN) +(?:void|(?:struct |const |enum )?\S+) +\**(\S+) +\(/
# The second nm|grep checks for file-scope variables with `extern' scope.
# Without gnulib's progname module, you might put program_name here.
-_gl_TS_unmarked_extern_vars ?=
+_gl_TS_unmarked_extern_vars ?= __.*
# NOTE: the _match variables are perl expressions -- not mere regular
# expressions -- so that you can extend them to match other patterns