From 0af0b7d87deb57060937e7f3e1887d7bee07c469 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A1n=20Tomko?= Date: Thu, 19 Aug 2021 14:01:14 +0200 Subject: [PATCH] syntax-check: drop useless useless-if-before-free MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit With most of new code using g_auto for cleanup, contributors are used to most of the free fucntions 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 Reviewed-by: Michal Privoznik --- 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 00091d5f8d..5d956c183f 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -114,181 +114,6 @@ _test_script_regex = \ 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='\. -# -# 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 < 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 = )) - { - # 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.47.2