[libvirt PATCH 4/5] syntax-check: drop useless useless-if-before-free
Ján Tomko
jtomko at redhat.com
Thu Aug 19 14:12:31 UTC 2021
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 at 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
More information about the libvir-list
mailing list