[libvirt PATCH 4/5] syntax-check: drop useless useless-if-before-free

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(a)redhat.com&gt; --- 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