[libvirt PATCH 0/5] Waste less CPU on syntax-check

Use 'git ls-tree' directly instead of a wrapper script that can possibly handle all the other versioning systems we do not use. Instead of adding, removing, then re-adding the absolute path prefix, only add it once per every syntax check rule. Delete the useless-if-before-free check since we've been ignoring its output anyway. This speeds up 'ninja test' on my 8-core laptop from: 19.781 s ± 0.174 s to 15.476 s ± 1.302 s Ján Tomko (5): syntax-check: assume we're using git syntax-check: use VC_LIST_EXCEPT instead of VC_LIST syntax-check: replace vc-list-files syntax-check: drop useless useless-if-before-free syntax-check: remove _test_script_regex build-aux/syntax-check.mk | 218 ++--------------------------- build-aux/useless-if-before-free | 226 ------------------------------- build-aux/vc-list-files | 113 ---------------- 3 files changed, 9 insertions(+), 548 deletions(-) delete mode 100755 build-aux/useless-if-before-free delete mode 100755 build-aux/vc-list-files -- 2.31.1

Meson already checks whether we're using git before running syntax check. This only affects direct invocation through make. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- build-aux/syntax-check.mk | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index cedb360bfd..e523c0bf3f 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -37,9 +37,6 @@ _sp = $(_empty) $(_empty) # If S1 == S2, return S1, otherwise the empty string. _equal = $(and $(findstring $(1),$(2)),$(findstring $(2),$(1))) -GIT = git -VC = $(GIT) - VC_LIST = $(top_srcdir)/build-aux/vc-list-files -C $(top_srcdir) # You can override this variable in syntax-check.mk to set your own regexp @@ -85,15 +82,6 @@ syntax-check-rules := $(sort $(shell $(SED) -n \ 's/^\(sc_[a-zA-Z0-9_-]*\):.*/\1/p' $(top_srcdir)/$(ME) $(_cfg_mk))) .PHONY: $(syntax-check-rules) -ifeq ($(shell $(VC_LIST) >/dev/null 2>&1; echo $$?),0) - local-checks-available += $(syntax-check-rules) -else - local-checks-available += no-vc-detected -no-vc-detected: - @echo "No version control files detected; skipping syntax check" -endif -.PHONY: $(local-checks-available) - # Arrange to print the name of each syntax-checking rule just before running it. $(syntax-check-rules): %: %.m sc_m_rules_ = $(patsubst %, %.m, $(syntax-check-rules)) @@ -116,7 +104,7 @@ $(sc_z_rules_): %.z: % # that computes and prints elapsed time. local-check := \ $(patsubst sc_%, sc_%.z, \ - $(filter-out $(local-checks-to-skip), $(local-checks-available))) + $(filter-out $(local-checks-to-skip), $(syntax-check-rules))) syntax-check: $(local-check) -- 2.31.1

Three callers were using VC_LIST directly. This is not wrong, because they exclude the always-excluded files by only looking for C and/or header files. But using VC_LIST here prevents switching it to outputting relative paths. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- build-aux/syntax-check.mk | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index e523c0bf3f..307255c300 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -1689,16 +1689,16 @@ sc_prohibit-duplicate-header: $(PYTHON) $(top_srcdir)/scripts/prohibit-duplicate-header.py sc_spacing-check: - $(AM_V_GEN)$(VC_LIST) | $(GREP) '\.c$$' | xargs \ + $(AM_V_GEN)$(VC_LIST_EXCEPT) | $(GREP) '\.c$$' | xargs \ $(PERL) $(top_srcdir)/build-aux/check-spacing.pl || \ { echo '$(ME): incorrect formatting' 1>&2; exit 1; } sc_mock-noinline: - $(AM_V_GEN)$(VC_LIST) | $(GREP) '\.[ch]$$' | $(RUNUTF8) xargs \ + $(AM_V_GEN)$(VC_LIST_EXCEPT) | $(GREP) '\.[ch]$$' | $(RUNUTF8) xargs \ $(PYTHON) $(top_srcdir)/scripts/mock-noinline.py sc_header-ifdef: - $(AM_V_GEN)$(VC_LIST) | $(GREP) '\.[h]$$' | $(RUNUTF8) xargs \ + $(AM_V_GEN)$(VC_LIST_EXCEPT) | $(GREP) '\.[h]$$' | $(RUNUTF8) xargs \ $(PYTHON) $(top_srcdir)/scripts/header-ifdef.py sc_group-qemu-caps: -- 2.31.1

Directly invoke git ls-tree instead of the wrapper file which also: * checks for other versioning systems * prepends the source directory to all output lines Since there is no srcdir prefix in the output anymore, also drop the extra 'sed' invocation that removes it. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- build-aux/syntax-check.mk | 6 +- build-aux/vc-list-files | 113 -------------------------------------- 2 files changed, 3 insertions(+), 116 deletions(-) delete mode 100755 build-aux/vc-list-files diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 307255c300..f493d19dae 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -37,7 +37,8 @@ _sp = $(_empty) $(_empty) # If S1 == S2, return S1, otherwise the empty string. _equal = $(and $(findstring $(1),$(2)),$(findstring $(2),$(1))) -VC_LIST = $(top_srcdir)/build-aux/vc-list-files -C $(top_srcdir) +VC_LIST = (cd $(top_srcdir); git ls-tree -r 'HEAD:' | \ + sed -n "s|^100[^ ]*.||p") # You can override this variable in syntax-check.mk to set your own regexp # matching files to ignore. @@ -63,8 +64,7 @@ endif _sc_excl = \ $(or $(exclude_file_name_regexp--$@),^$$) VC_LIST_EXCEPT = \ - $(VC_LIST) | $(SED) 's|^$(_dot_escaped_srcdir)/||' \ - | $(GREP) -Ev -e '($(VC_LIST_ALWAYS_EXCLUDE_REGEX)|$(_sc_excl))' \ + $(VC_LIST) | $(GREP) -Ev -e '($(VC_LIST_ALWAYS_EXCLUDE_REGEX)|$(_sc_excl))' \ $(_prepend_srcdir_prefix) # Prevent programs like 'sort' from considering distinct strings to be equal. diff --git a/build-aux/vc-list-files b/build-aux/vc-list-files deleted file mode 100755 index af6b1c0f8a..0000000000 --- a/build-aux/vc-list-files +++ /dev/null @@ -1,113 +0,0 @@ -#!/bin/sh -# List version-controlled file names. - -# Print a version string. -scriptversion=2018-03-07.03; # UTC - -# Copyright (C) 2006-2019 Free Software Foundation, Inc. - -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. - -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. - -# You should have received a copy of the GNU General Public License -# along with this program. If not, see <https://www.gnu.org/licenses/>. - - -# List the specified version-controlled files. -# With no argument, list them all. With a single DIRECTORY argument, -# list the version-controlled files in that directory. - -# If there's an argument, it must be a single, "."-relative directory name. -# cvsu is part of the cvsutils package: http://www.red-bean.com/cvsutils/ - -postprocess= -case $1 in - --help) cat <<EOF -Usage: $0 [-C SRCDIR] [DIR...] - -Output a list of version-controlled files in DIR (default .), relative to -SRCDIR (default .). SRCDIR must be the top directory of a checkout. - -Options: - --help print this help, then exit - --version print version number, then exit - -C SRCDIR change directory to SRCDIR before generating list - -Report bugs and patches to <bug-gnulib@gnu.org>. -EOF - exit ;; - - --version) - year=`echo "$scriptversion" | sed 's/[^0-9].*//'` - cat <<EOF -vc-list-files $scriptversion -Copyright (C) $year Free Software Foundation, Inc, -License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html> -This is free software: you are free to change and redistribute it. -There is NO WARRANTY, to the extent permitted by law. -EOF - exit ;; - - -C) - test "$2" = . || postprocess="| sed 's|^|$2/|'" - cd "$2" || exit 1 - shift; shift ;; -esac - -test $# = 0 && set . - -for dir -do - if test -d .git || test -f .git; then - test "x$dir" = x. \ - && dir= sed_esc= \ - || { dir="$dir/"; sed_esc=`echo "$dir"|env sed 's,\([\\/]\),\\\\\1,g'`; } - # Ignore git symlinks - either they point into the tree, in which case - # we don't need to visit the target twice, or they point somewhere - # else (often into a submodule), in which case the content does not - # belong to this package. - eval exec git ls-tree -r 'HEAD:"$dir"' \ - \| sed -n '"s/^100[^ ]*./$sed_esc/p"' $postprocess - elif test -d .hg; then - eval exec hg locate '"$dir/*"' $postprocess - elif test -d .bzr; then - test "$postprocess" = '' && postprocess="| sed 's|^\./||'" - eval exec bzr ls -R --versioned '"$dir"' $postprocess - elif test -d CVS; then - test "$postprocess" = '' && postprocess="| sed 's|^\./||'" - if test -x build-aux/cvsu; then - eval build-aux/cvsu --find --types=AFGM '"$dir"' $postprocess - elif (cvsu --help) >/dev/null 2>&1; then - eval cvsu --find --types=AFGM '"$dir"' $postprocess - else - eval awk -F/ \''{ \ - if (!$1 && $3 !~ /^-/) { \ - f=FILENAME; \ - if (f ~ /CVS\/Entries$/) \ - f = substr(f, 1, length(f)-11); \ - print f $2; \ - }}'\'' \ - `find "$dir" -name Entries -print` /dev/null' $postprocess - fi - elif test -d .svn; then - eval exec svn list -R '"$dir"' $postprocess - else - echo "$0: Failed to determine type of version control used in `pwd`" 1>&2 - exit 1 - fi -done - -# Local variables: -# eval: (add-hook 'before-save-hook 'time-stamp) -# time-stamp-start: "scriptversion=" -# time-stamp-format: "%:y-%02m-%02d.%02H" -# time-stamp-time-zone: "UTC0" -# time-stamp-end: "; # UTC" -# End: -- 2.31.1

With most of new code using g_auto for cleanup, contributors are used to most of the free fucntion handling NULL gracefully. Also, despite finding some occurrences in current codebase: avoid_if_before_free ~/libvirt/src/ch/ch_monitor.c: if (mon->vm) virObjectUnref(mon->vm); ~/libvirt/src/util/virresctrl.c: if (a_type->masks[cache]) virBitmapFree(a_type->masks[cache]); the check passes succesfully, because the script's logic: Exit status: 0 one or more matches 1 no match 2 an error does not play nicely with xargs: xargs exits with the following status: 0 if it succeeds 123 if any invocation of the command exited with status 1-125 The list of functions is also out of date - e.g. qemuCapsFree has been renamed since. This also helps eliminate one more Perl script per our programming languages strategy: https://libvirt.org/programming-languages.html Signed-off-by: Ján Tomko <jtomko@redhat.com> --- build-aux/syntax-check.mk | 190 +------------------------- build-aux/useless-if-before-free | 226 ------------------------------- 2 files changed, 2 insertions(+), 414 deletions(-) delete mode 100755 build-aux/useless-if-before-free diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index f493d19dae..b72a5b93d8 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -114,181 +114,6 @@ _test_script_regex = \<test-lib\.sh\> VC_LIST_ALWAYS_EXCLUDE_REGEX = \ (^(docs/(news(-[0-9]*)?\.html\.in|.*\.patch))|\.(po|fig|gif|ico|png))$$ -# Functions like free() that are no-ops on NULL arguments. -useless_free_options = \ - --name=VBOX_UTF16_FREE \ - --name=VBOX_UTF8_FREE \ - --name=VBOX_COM_UNALLOC_MEM \ - --name=VIR_FREE \ - --name=qemuCapsFree \ - --name=qemuMigrationCookieFree \ - --name=qemuMigrationCookieGraphicsFree \ - --name=sexpr_free \ - --name=usbFreeDevice \ - --name=virBandwidthDefFree \ - --name=virBitmapFree \ - --name=virCPUDefFree \ - --name=virCapabilitiesFree \ - --name=virCapabilitiesFreeGuest \ - --name=virCapabilitiesFreeGuestDomain \ - --name=virCapabilitiesFreeGuestFeature \ - --name=virCapabilitiesFreeGuestMachine \ - --name=virCapabilitiesFreeHostNUMACell \ - --name=virCapabilitiesFreeMachines \ - --name=virCgroupFree \ - --name=virCommandFree \ - --name=virConfFreeList \ - --name=virConfFreeValue \ - --name=virDomainActualNetDefFree \ - --name=virDomainChrDefFree \ - --name=virDomainControllerDefFree \ - --name=virDomainDefFree \ - --name=virDomainDeviceDefFree \ - --name=virDomainDiskDefFree \ - --name=virDomainEventCallbackListFree \ - --name=virObjectEventQueueFree \ - --name=virDomainFSDefFree \ - --name=virDomainGraphicsDefFree \ - --name=virDomainHostdevDefFree \ - --name=virDomainInputDefFree \ - --name=virDomainNetDefFree \ - --name=virDomainObjFree \ - --name=virDomainSmartcardDefFree \ - --name=virDomainSnapshotObjFree \ - --name=virDomainSoundDefFree \ - --name=virDomainVideoDefFree \ - --name=virDomainWatchdogDefFree \ - --name=virFileDirectFdFree \ - --name=virHashFree \ - --name=virInterfaceDefFree \ - --name=virInterfaceIpDefFree \ - --name=virInterfaceObjFree \ - --name=virInterfaceProtocolDefFree \ - --name=virJSONValueFree \ - --name=virLastErrFreeData \ - --name=virNetMessageFree \ - --name=virNWFilterDefFree \ - --name=virNWFilterEntryFree \ - --name=virNWFilterHashTableFree \ - --name=virNWFilterIPAddrLearnReqFree \ - --name=virNWFilterIncludeDefFree \ - --name=virNWFilterObjFree \ - --name=virNWFilterRuleDefFree \ - --name=virNWFilterRuleInstFree \ - --name=virNetworkDefFree \ - --name=virNodeDeviceDefFree \ - --name=virNodeDeviceObjFree \ - --name=virObjectUnref \ - --name=virObjectFreeCallback \ - --name=virPCIDeviceFree \ - --name=virSecretDefFree \ - --name=virStorageEncryptionFree \ - --name=virStorageEncryptionSecretFree \ - --name=virStorageFileFreeMetadata \ - --name=virStoragePoolDefFree \ - --name=virStoragePoolObjFree \ - --name=virStoragePoolSourceFree \ - --name=virStorageVolDefFree \ - --name=virThreadPoolFree \ - --name=xmlBufferFree \ - --name=xmlFree \ - --name=xmlFreeDoc \ - --name=xmlFreeNode \ - --name=xmlXPathFreeContext \ - --name=xmlXPathFreeObject - -# The following template was generated by this command: -# make ID && aid free|grep '^vi'|sed 's/ .*//;s/^/# /' -# N virBufferFreeAndReset -# y virCPUDefFree -# y virCapabilitiesFree -# y virCapabilitiesFreeGuest -# y virCapabilitiesFreeGuestDomain -# y virCapabilitiesFreeGuestFeature -# y virCapabilitiesFreeGuestMachine -# y virCapabilitiesFreeHostNUMACell -# y virCapabilitiesFreeMachines -# N virCapabilitiesFreeNUMAInfo FIXME -# y virCgroupFree -# N virConfFree (diagnoses the "error") -# y virConfFreeList -# y virConfFreeValue -# y virDomainChrDefFree -# y virDomainControllerDefFree -# y virDomainDefFree -# y virDomainDeviceDefFree -# y virDomainDiskDefFree -# y virDomainEventCallbackListFree -# y virDomainEventQueueFree -# y virDomainFSDefFree -# n virDomainFree -# n virDomainFreeName (can't fix -- returns int) -# y virDomainGraphicsDefFree -# y virDomainHostdevDefFree -# y virDomainInputDefFree -# y virDomainNetDefFree -# y virDomainObjFree -# n virDomainSnapshotFree (returns int) -# n virDomainSnapshotFreeName (returns int) -# y virDomainSnapshotObjFree -# y virDomainSoundDefFree -# y virDomainVideoDefFree -# y virDomainWatchdogDefFree -# n virDrvNodeGetCellsFreeMemory (returns int) -# n virDrvNodeGetFreeMemory (returns long long) -# n virFree - dereferences param -# n virFreeError -# n virHashFree (takes 2 args) -# y virInterfaceDefFree -# n virInterfaceFree (returns int) -# n virInterfaceFreeName -# y virInterfaceIpDefFree -# y virInterfaceObjFree -# n virInterfaceObjListFree -# y virInterfaceProtocolDefFree -# y virJSONValueFree -# y virLastErrFreeData -# y virNWFilterDefFree -# y virNWFilterEntryFree -# n virNWFilterFree (returns int) -# y virNWFilterHashTableFree -# y virNWFilterIPAddrLearnReqFree -# y virNWFilterIncludeDefFree -# n virNWFilterFreeName (returns int) -# y virNWFilterObjFree -# n virNWFilterObjListFree FIXME -# y virNWFilterRuleDefFree -# n virNWFilterRuleFreeInstanceData (typedef) -# y virNWFilterRuleInstFree -# y virNetworkDefFree -# n virNetworkFree (returns int) -# n virNetworkFreeName (returns int) -# n virNodeDevCapsDefFree FIXME -# y virNodeDeviceDefFree -# n virNodeDeviceFree (returns int) -# y virNodeDeviceObjFree -# n virNodeDeviceObjListFree FIXME -# n virNodeGetCellsFreeMemory (returns int) -# n virNodeGetFreeMemory (returns non-void) -# y virSecretDefFree -# n virSecretFree (returns non-void) -# n virSecretFreeName (2 args) -# n virSecurityLabelDefFree FIXME -# n virStorageBackendDiskMakeFreeExtent (returns non-void) -# y virStorageEncryptionFree -# y virStorageEncryptionSecretFree -# n virStorageFreeType (enum) -# y virStoragePoolDefFree -# n virStoragePoolFree (returns non-void) -# n virStoragePoolFreeName (returns non-void) -# y virStoragePoolObjFree -# n virStoragePoolObjListFree FIXME -# y virStoragePoolSourceFree -# y virStorageVolDefFree -# n virStorageVolFree (returns non-void) -# n virStorageVolFreeName (returns non-void) -# n virStreamFree - # Avoid uses of write(2). Either switch to streams (fwrite), or use # the safewrite wrapper. sc_avoid_write: @@ -1250,17 +1075,6 @@ define _sc_search_regexp fi || :; endef -sc_avoid_if_before_free: - @$(VC_LIST_EXCEPT) \ - | $(GREP) -v useless-if-before-free \ - | xargs \ - $(top_srcdir)/build-aux/useless-if-before-free \ - $(useless_free_options) \ - && { printf '$(ME): found useless "if"' \ - ' before "free" above\n' 1>&2; \ - exit 1; } \ - || : - sc_cast_of_argument_to_free: @prohibit='\<free *\( *\(' halt="don't cast free argument" \ $(_sc_search_regexp) @@ -1777,7 +1591,7 @@ exclude_file_name_regexp--sc_prohibit_canonicalize_file_name = \ ^(build-aux/syntax-check\.mk|tests/virfilemock\.c)$$ exclude_file_name_regexp--sc_prohibit_raw_allocation = \ - ^(docs/advanced-tests\.rst|src/util/viralloc\.[ch]|examples/.*|tests/(securityselinuxhelper|(vircgroup|nss)mock|commandhelper)\.c|tools/wireshark/src/packet-libvirt\.c|tools/nss/libvirt_nss(_leases|_macs)?\.c|build-aux/useless-if-before-free)$$ + ^(docs/advanced-tests\.rst|src/util/viralloc\.[ch]|examples/.*|tests/(securityselinuxhelper|(vircgroup|nss)mock|commandhelper)\.c|tools/wireshark/src/packet-libvirt\.c|tools/nss/libvirt_nss(_leases|_macs)?\.c)$$ exclude_file_name_regexp--sc_prohibit_readlink = \ ^src/(util/virutil|lxc/lxc_container)\.c$$ @@ -1793,7 +1607,7 @@ exclude_file_name_regexp--sc_prohibit_xmlGetProp = ^src/util/virxml\.c$$ exclude_file_name_regexp--sc_prohibit_xmlURI = ^src/util/viruri\.c$$ -exclude_file_name_regexp--sc_prohibit_return_as_function = \.py|build-aux/useless-if-before-free$$ +exclude_file_name_regexp--sc_prohibit_return_as_function = \.py$$ exclude_file_name_regexp--sc_require_config_h = \ ^(examples/|tools/virsh-edit\.c$$|tests/virmockstathelpers.c) diff --git a/build-aux/useless-if-before-free b/build-aux/useless-if-before-free deleted file mode 100755 index 6ac8aa9196..0000000000 --- a/build-aux/useless-if-before-free +++ /dev/null @@ -1,226 +0,0 @@ -#!/bin/sh -#! -*-perl-*- - -# Detect instances of "if (p) free (p);". -# Likewise "if (p != 0)", "if (0 != p)", or with NULL; and with braces. - -# Copyright (C) 2008-2019 Free Software Foundation, Inc. -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see <https://www.gnu.org/licenses/>. -# -# Written by Jim Meyering - -# This is a prologue that allows to run a perl script as an executable -# on systems that are compliant to a POSIX version before POSIX:2017. -# On such systems, the usual invocation of an executable through execlp() -# or execvp() fails with ENOEXEC if it is a script that does not start -# with a #! line. The script interpreter mentioned in the #! line has -# to be /bin/sh, because on GuixSD systems that is the only program that -# has a fixed file name. The second line is essential for perl and is -# also useful for editing this file in Emacs. The next two lines below -# are valid code in both sh and perl. When executed by sh, they re-execute -# the script through the perl program found in $PATH. The '-x' option -# is essential as well; without it, perl would re-execute the script -# through /bin/sh. When executed by perl, the next two lines are a no-op. -eval 'exec perl -wSx "$0" "$@"' - if 0; - -my $VERSION = '2018-03-07 03:47'; # UTC -# The definition above must lie within the first 8 lines in order -# for the Emacs time-stamp write hook (at end) to update it. -# If you change this file with Emacs, please let the write hook -# do its job. Otherwise, update this string manually. - -use strict; -use warnings; -use Getopt::Long; - -(my $ME = $0) =~ s|.*/||; - -# use File::Coda; # https://meyering.net/code/Coda/ -END { - defined fileno STDOUT or return; - close STDOUT and return; - warn "$ME: failed to close standard output: $!\n"; - $? ||= 1; -} - -sub usage ($) -{ - my ($exit_code) = @_; - my $STREAM = ($exit_code == 0 ? *STDOUT : *STDERR); - if ($exit_code != 0) - { - print $STREAM "Try '$ME --help' for more information.\n"; - } - else - { - print $STREAM <<EOF; -Usage: $ME [OPTIONS] FILE... - -Detect any instance in FILE of a useless "if" test before a free call, e.g., -"if (p) free (p);". Any such test may be safely removed without affecting -the semantics of the C code in FILE. Use --name=FOO --name=BAR to also -detect free-like functions named FOO and BAR. - -OPTIONS: - - --list print only the name of each matching FILE (\\0-terminated) - --name=N add name N to the list of \'free\'-like functions to detect; - may be repeated - - --help display this help and exit - --version output version information and exit - -Exit status: - - 0 one or more matches - 1 no match - 2 an error - -EXAMPLE: - -For example, this command prints all removable "if" tests before "free" -and "kfree" calls in the linux kernel sources: - - git ls-files -z |xargs -0 $ME --name=kfree - -EOF - } - exit $exit_code; -} - -sub is_NULL ($) -{ - my ($expr) = @_; - return ($expr eq 'NULL' || $expr eq '0'); -} - -{ - sub EXIT_MATCH {0} - sub EXIT_NO_MATCH {1} - sub EXIT_ERROR {2} - my $err = EXIT_NO_MATCH; - - my $list; - my @name = qw(free); - GetOptions - ( - help => sub { usage 0 }, - version => sub { print "$ME version $VERSION\n"; exit }, - list => \$list, - 'name=s@' => \@name, - ) or usage 1; - - # Make sure we have the right number of non-option arguments. - # Always tell the user why we fail. - @ARGV < 1 - and (warn "$ME: missing FILE argument\n"), usage EXIT_ERROR; - - my $or = join '|', @name; - my $regexp = qr/(?:$or)/; - - # Set the input record separator. - # Note: this makes it impractical to print line numbers. - $/ = '"'; - - my $found_match = 0; - FILE: - foreach my $file (@ARGV) - { - open FH, '<', $file - or (warn "$ME: can't open '$file' for reading: $!\n"), - $err = EXIT_ERROR, next; - while (defined (my $line = <FH>)) - { - # Skip non-matching lines early to save time - $line =~ /\bif\b/ - or next; - while ($line =~ - /\b(if\s*\(\s*([^)]+?)(?:\s*!=\s*([^)]+?))?\s*\) - # 1 2 3 - (?: \s*$regexp\s*\((?:\s*\([^)]+\))?\s*([^)]+)\)\s*;| - \s*\{\s*$regexp\s*\((?:\s*\([^)]+\))?\s*([^)]+)\)\s*;\s*\}))/sxg) - { - my $all = $1; - my ($lhs, $rhs) = ($2, $3); - my ($free_opnd, $braced_free_opnd) = ($4, $5); - my $non_NULL; - if (!defined $rhs) { $non_NULL = $lhs } - elsif (is_NULL $rhs) { $non_NULL = $lhs } - elsif (is_NULL $lhs) { $non_NULL = $rhs } - else { next } - - # Compare the non-NULL part of the "if" expression and the - # free'd expression, without regard to white space. - $non_NULL =~ tr/ \t//d; - my $e2 = defined $free_opnd ? $free_opnd : $braced_free_opnd; - $e2 =~ tr/ \t//d; - if ($non_NULL eq $e2) - { - $found_match = 1; - $list - and (print "$file\0"), next FILE; - print "$file: $all\n"; - } - } - } - } - continue - { - close FH; - } - - $found_match && $err == EXIT_NO_MATCH - and $err = EXIT_MATCH; - - exit $err; -} - -my $foo = <<'EOF'; -# The above is to *find* them. -# This adjusts them, removing the unnecessary "if (p)" part. - -# FIXME: do something like this as an option (doesn't do braces): -free=xfree -git grep -l -z "$free *(" \ - | xargs -0 useless-if-before-free -l --name="$free" \ - | xargs -0 perl -0x3b -pi -e \ - 's/\bif\s*\(\s*(\S+?)(?:\s*!=\s*(?:0|NULL))?\s*\)\s+('"$free"'\s*\((?:\s*\([^)]+\))?\s*\1\s*\)\s*;)/$2/s' - -# Use the following to remove redundant uses of kfree inside braces. -# Note that -0777 puts perl in slurp-whole-file mode; -# but we have plenty of memory, these days... -free=kfree -git grep -l -z "$free *(" \ - | xargs -0 useless-if-before-free -l --name="$free" \ - | xargs -0 perl -0777 -pi -e \ - 's/\bif\s*\(\s*(\S+?)(?:\s*!=\s*(?:0|NULL))?\s*\)\s*\{\s*('"$free"'\s*\((?:\s*\([^)]+\))?\s*\1\s*\);)\s*\}[^\n]*$/$2/gms' - -Be careful that the result of the above transformation is valid. -If the matched string is followed by "else", then obviously, it won't be. - -When modifying files, refuse to process anything other than a regular file. -EOF - -## Local Variables: -## mode: perl -## indent-tabs-mode: nil -## eval: (add-hook 'before-save-hook 'time-stamp) -## time-stamp-line-limit: 50 -## time-stamp-start: "my $VERSION = '" -## time-stamp-format: "%:y-%02m-%02d %02H:%02M" -## time-stamp-time-zone: "UTC0" -## time-stamp-end: "'; # UTC" -## End: -- 2.31.1

Commit abab5c47f8060eddf0212084a1d70c4b1e3d92dd incorrectly assumed we do not have any files that could be affected by sc_prohibit_reversed_compare_failure due to the conditional assignment: _test_script_regex ?= \<init\.sh\> so it removed the check. Also remove the leftover assignment of test-lib.sh, since any new code attemptint to use the compare function with reversed arguments should be rejected by review for using shell instead of C or Python. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- build-aux/syntax-check.mk | 2 -- 1 file changed, 2 deletions(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index b72a5b93d8..cf7a1fa4c6 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -108,8 +108,6 @@ local-check := \ syntax-check: $(local-check) -_test_script_regex = \<test-lib\.sh\> - # Files that should never cause syntax check failures. VC_LIST_ALWAYS_EXCLUDE_REGEX = \ (^(docs/(news(-[0-9]*)?\.html\.in|.*\.patch))|\.(po|fig|gif|ico|png))$$ -- 2.31.1

On 8/19/21 4:12 PM, Ján Tomko wrote:
Use 'git ls-tree' directly instead of a wrapper script that can possibly handle all the other versioning systems we do not use.
Instead of adding, removing, then re-adding the absolute path prefix, only add it once per every syntax check rule.
Delete the useless-if-before-free check since we've been ignoring its output anyway.
This speeds up 'ninja test' on my 8-core laptop from: 19.781 s ± 0.174 s to 15.476 s ± 1.302 s
Ján Tomko (5): syntax-check: assume we're using git syntax-check: use VC_LIST_EXCEPT instead of VC_LIST syntax-check: replace vc-list-files syntax-check: drop useless useless-if-before-free syntax-check: remove _test_script_regex
build-aux/syntax-check.mk | 218 ++--------------------------- build-aux/useless-if-before-free | 226 ------------------------------- build-aux/vc-list-files | 113 ---------------- 3 files changed, 9 insertions(+), 548 deletions(-) delete mode 100755 build-aux/useless-if-before-free delete mode 100755 build-aux/vc-list-files
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Ján Tomko
-
Michal Prívozník