According to jema...@gnu.org on 2/25/2010 4:26 PM:
> I heavily expanded the _prohibit_regexp macro to accept more arguments
> and renamed it to _sc_search_regexp. Using the new parameters it is
> possible to write rules like:
>
> # Ensure that each .c file containing a "main" function also
> # calls set_program_name.
> sc_program_name:
> @require='set_program_name *\(m?argv\[0\]\);' \
> in_files='\.c$$' \
> containing='^main *(' \
> msg='the above files do not call set_program_name' \
> $(_sc_search_regexp)
Slick.
> +# _sc_search_regexp
> +#
> +# This macro searches for a given construct in the selected files and
> +# then takes some action.
> +#
> +# Parameters (environment variables):
Shell variables, actually; we don't require that they be exported to the
environment.
> +#
> +# prohibit | require
> +#
> +# Regular expression denoting either a forbidded construct or a
s/forbidded/forbidden/. It would be worth clarifying BRE or ERE.
> +# required construct. Those arguments are exclusive.
> +#
> +# in_files | not_in_files
> +#
> +# grep-E-style regexp denoting the files to check.
> +#
> +# containing | non_containing
> +#
> +# Select the files (non) containing strings matching this regexp.
> +# If both arguments are specified then CONTAINING takes
> +# precedence.
> +#
> +# with_grep_options
> +#
> +# Extra options for grep.
> +#
> +# ignore_case
> +#
> +# Ignore case.
> +#
> +# halt | warn
> +#
> +# Message to display before to halt the execution.
s/halt the execution/halting execution or warning/
> +
> +# By default, _sc_search_regexp does not ignore case.
> export ignore_case =
> _ignore_case = $$(test -n "$$ignore_case" && echo -i || :)
>
> -# There are many rules below that prohibit constructs in this package.
> -# If the offending construct can be matched with a grep-E-style regexp,
> -# use this macro. The shell variables "re" and "msg" must be defined.
> -define _prohibit_regexp
> +define _sc_say_and_exit
> + dummy=; : so we do not need a semicolon before each use;
> \
> + { echo "$(ME): $$msg" 1>&2; exit 1; };
> +endef
> +
> +define _sc_search_regexp
> + dummy=; : so we do not need a semicolon before each use;
> \
> +
> \
> + : Check arguments;
> \
Line up the \. (I wonder if my MUA will botch line endings when sending
this reply).
> + test -n "$$prohibit" -a -n "$$require" &&
> \
'test -a' is not portable, and POSIX 2008 recommends against its use.
It's not that hard to get in the habit of using 'test && test' instead.
> + { msg='Cannot specify both prohibit and require' $(_sc_say_and_exit) }
> || :; \
> + test -z "$$prohibit" -a -z "$$require" &&
> \
> + { msg='Should specify either prohibit or require' $(_sc_say_and_exit) }
> || :; \
> + test -n "$$in_files" -a -n "$$not_in_files" &&
> \
> + { msg='Cannot specify both in-files and not_in_files'
> \
> + $(_sc_say_and_exit) } || :;
> \
> + test "x$$msg" != x || { msg='msg not defined' $(_sc_say_and_exit) };
> \
> +
> \
> + : Filter by file name;
> \
> + if test -n "$$in_files"; then
> \
> + files=$$($(VC_LIST_EXCEPT) | grep -E "$$in_files");
> \
> + else
> \
> + files=$$($(VC_LIST_EXCEPT));
> \
> + fi;
> \
> +
> \
> + : Filter by content;
> \
> + test -n "$$files" -a -n "$$containing" &&
> \
> + { files=$$(grep -l "$$containing" $$files); } || :;
> \
> + test -n "$$files" -a -n "$$non_containing" &&
> \
> + { files=$$(grep -vl "$$non_containing" $$files); } || :;
> \
> +
> \
> + : Check for the construct;
> \
> + if test -n "$$files"; then
> \
> + if test -n "$$prohibit"; then
> \
> + grep $$with_grep_options $(_ignore_case) -nE "$$prohibit" $$files &&
> \
> + { msg="$$msg" $(_sc_say_and_exit) } || :;
> \
> + else
> \
> + grep $$with_grep_options $(_ignore_case) -LE "$$require" $$files | grep
> . && \
> + { msg="$$msg" $(_sc_say_and_exit) } || :;
> \
> + fi
> \
> + else :;
> \
> + fi || :;
> +endef
> +
> +# To use this "command" macro, you must first define two shell variables:
> +# h: the header, enclosed in <> or ""
> +# re: a regular expression that matches IFF something provided by $h is used.
> +define _sc_header_without_use
> dummy=; : so we do not need a semicolon before each use; \
> - test "x$$re" != x || { echo '$(ME): re not defined' 1>&2; exit 1; };
> \
> - test "x$$msg" != x || { echo '$(ME): msg not defined' 1>&2; exit 1; };\
> - grep $(_ignore_case) -nE "$$re" $$($(VC_LIST_EXCEPT)) && \
> - { echo '$(ME): '"$$msg" 1>&2; exit 1; } || :
> + h_esc=`echo "$$h"|sed 's/\./\\\\./g'`; \
> + if $(VC_LIST_EXCEPT) | grep -l '\.c$$' > /dev/null; then \
> + files=$$(grep -l '^# *include '"$$h_esc" \
> + $$($(VC_LIST_EXCEPT) | grep '\.c$$')) && \
> + grep -LE "$$re" $$files | grep . &&
> \
> + { echo "$(ME): the above files include $$h but don't use it" \
> + 1>&2; exit 1; } || :; \
> + else :; \
> + fi
> endef
Lots of decent looking changes in the rest of the file, but I didn't look
closely. I did, however, spot some things that might warrant a resubmission:
>
> sc_avoid_if_before_free:
> # Error messages should not end with a period
> sc_error_message_period:
> - @grep -nEA2 '[^rp]error \(' $$($(VC_LIST_EXCEPT)) \
> + @grep -nEA2 '[^rp]error *\(' $$($(VC_LIST_EXCEPT)) \
Changes like this are worth an independent patch. 'git add -i' can be
helpful in splitting a patch.
> sc_prohibit_openat_without_use:
> @h='"openat.h"' \
>
> re='\<(openat_(permissive|needs_fchdir|(save|restore)_fail)|l?(stat|ch(own|mod))at|(euid)?accessat)\>'
> \
> - $(_header_without_use)
> + $(_sc_header_without_use)
For that matter, changes like this could be done in stages for easier
reviewing; one patch to rename _{,sc_}header_without_use, and another one
to redefine _sc_header_without_use in terms of your new powerhouse macro.
--
Eric Blake ebl...@redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
