Various checks in the code were under-indented relative to other
surrounding code.
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
scripts/checkpatch.pl | 98 +++++++++++++++++++++----------------------
1 file changed, 49 insertions(+), 49 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d355c0dad5..7675418b0b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1681,19 +1681,19 @@ sub process {
# Check SPDX-License-Identifier references a permitted license
if ($rawline =~ m,SPDX-License-Identifier: (.*?)(\*/)?\s*$,) {
- &checkspdx($realfile, $1);
+ &checkspdx($realfile, $1);
}
if ($rawline =~ m,(SPDX-[a-zA-Z0-9-_]+):,) {
- my $tag = $1;
- my @permitted = qw(
- SPDX-License-Identifier
- );
-
- unless (grep { /^$tag$/ } @permitted) {
- ERROR("Tag $tag not permitted in QEMU code, valid " .
- "choices are: " . join(", ", @permitted));
- }
+ my $tag = $1;
+ my @permitted = qw(
+ SPDX-License-Identifier
+ );
+
+ unless (grep { /^$tag$/ } @permitted) {
+ ERROR("Tag $tag not permitted in QEMU code, valid " .
+ "choices are: " . join(", ", @permitted));
+ }
}
# Check for wrappage within a valid hunk of the file
@@ -2274,7 +2274,7 @@ sub process {
# missing space after union, struct or enum definition
if ($line =~ /^.\s*(?:typedef\s+)?(enum|union|struct)(?:\s+$Ident)?(?:\s+$Ident)?[=\{]/) {
- ERROR("missing space after $1 definition\n" . $herecurr);
+ ERROR("missing space after $1 definition\n" . $herecurr);
}
# check for spacing round square brackets; allowed:
@@ -2569,7 +2569,7 @@ sub process {
if ($line =~ /^.\s*(Q(?:S?LIST|SIMPLEQ|TAILQ)_HEAD)\s*\(\s*[^,]/ &&
$line !~ /^.typedef/) {
- ERROR("named $1 should be typedefed separately\n" . $herecurr);
+ ERROR("named $1 should be typedefed separately\n" . $herecurr);
}
# Need a space before open parenthesis after if, while etc
@@ -3118,48 +3118,48 @@ sub process {
# Qemu error function tests
- # Find newlines in error messages
- my $qemu_error_funcs = qr{error_setg|
- error_setg_errno|
- error_setg_win32|
- error_setg_file_open|
- error_set|
- error_prepend|
- warn_reportf_err|
- error_reportf_err|
- error_vreport|
- warn_vreport|
- info_vreport|
- error_report|
- warn_report|
- info_report|
- g_test_message}x;
-
- if ($rawline =~ /\b(?:$qemu_error_funcs)\s*\(.*\".*\\n/) {
- ERROR("Error messages should not contain newlines\n" . $herecurr);
- }
+ # Find newlines in error messages
+ my $qemu_error_funcs = qr{error_setg|
+ error_setg_errno|
+ error_setg_win32|
+ error_setg_file_open|
+ error_set|
+ error_prepend|
+ warn_reportf_err|
+ error_reportf_err|
+ error_vreport|
+ warn_vreport|
+ info_vreport|
+ error_report|
+ warn_report|
+ info_report|
+ g_test_message}x;
+
+ if ($rawline =~ /\b(?:$qemu_error_funcs)\s*\(.*\".*\\n/) {
+ ERROR("Error messages should not contain newlines\n" . $herecurr);
+ }
- # Continue checking for error messages that contains newlines. This
- # check handles cases where string literals are spread over multiple lines.
- # Example:
- # error_report("Error msg line #1"
- # "Error msg line #2\n");
- my $quoted_newline_regex = qr{\+\s*\".*\\n.*\"};
- my $continued_str_literal = qr{\+\s*\".*\"};
+ # Continue checking for error messages that contains newlines. This
+ # check handles cases where string literals are spread over multiple lines.
+ # Example:
+ # error_report("Error msg line #1"
+ # "Error msg line #2\n");
+ my $quoted_newline_regex = qr{\+\s*\".*\\n.*\"};
+ my $continued_str_literal = qr{\+\s*\".*\"};
- if ($rawline =~ /$quoted_newline_regex/) {
- # Backtrack to first line that does not contain only a quoted literal
- # and assume that it is the start of the statement.
- my $i = $linenr - 2;
+ if ($rawline =~ /$quoted_newline_regex/) {
+ # Backtrack to first line that does not contain only a quoted literal
+ # and assume that it is the start of the statement.
+ my $i = $linenr - 2;
- while (($i >= 0) & $rawlines[$i] =~ /$continued_str_literal/) {
- $i--;
- }
+ while (($i >= 0) & $rawlines[$i] =~ /$continued_str_literal/) {
+ $i--;
+ }
- if ($rawlines[$i] =~ /\b(?:$qemu_error_funcs)\s*\(/) {
- ERROR("Error messages should not contain newlines\n" . $herecurr);
+ if ($rawlines[$i] =~ /\b(?:$qemu_error_funcs)\s*\(/) {
+ ERROR("Error messages should not contain newlines\n" . $herecurr);
+ }
}
- }
# check for non-portable libc calls that have portable alternatives in QEMU
if ($line =~ /\bffs\(/) {
--
2.49.0
Daniel P. Berrangé <berrange@redhat.com> writes: > Various checks in the code were under-indented relative to other > surrounding code. Are we accepting that we have hard forked from the Linux checkpatch now? <snip> -- Alex Bennée Virtualisation Tech Lead @ Linaro
On Mon, 19 May 2025 at 17:27, Alex Bennée <alex.bennee@linaro.org> wrote: > > Daniel P. Berrangé <berrange@redhat.com> writes: > > > Various checks in the code were under-indented relative to other > > surrounding code. > > Are we accepting that we have hard forked from the Linux checkpatch now? Yes, I think that ship sailed some years ago: we've never done more than try to port some specific kernel commits across to our version; we've not tried to do a sync to the kernel's version of the script ever since we first imported the script in 2011. thanks -- PMM
On Mon, May 19, 2025 at 05:27:21PM +0100, Alex Bennée wrote: > Daniel P. Berrangé <berrange@redhat.com> writes: > > > Various checks in the code were under-indented relative to other > > surrounding code. > > Are we accepting that we have hard forked from the Linux checkpatch now? The changed lines were all in QEMU specific checks, so don't make that situation any worse than it already is. AFAICT, we've got so many add-ons that we're a quite significant fork already. It may be possible to merge back changes from Linux still, but I wouldn't want to be the one figuring out that resolution :-) With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, 15 May 2025 at 14:59, Daniel P. Berrangé <berrange@redhat.com> wrote: > > Various checks in the code were under-indented relative to other > surrounding code. Isn't the problem here not that they're under-indented, but that they don't follow the "indent with hard coded tab characters" style that the rest of the script does? Looking at the patch on patchew it looks like these changes do make the code use hardcoded tabs, but I think it would be worth mentioning that in the commit message. (I assumed from the wording -- and also because my mail client was being misleading -- that these changes were adding 8-space indent.) Otherwise Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On Mon, May 19, 2025 at 01:12:09PM +0100, Peter Maydell wrote: > On Thu, 15 May 2025 at 14:59, Daniel P. Berrangé <berrange@redhat.com> wrote: > > > > Various checks in the code were under-indented relative to other > > surrounding code. > > Isn't the problem here not that they're under-indented, > but that they don't follow the "indent with hard coded > tab characters" style that the rest of the script does? > > Looking at the patch on patchew it looks like these changes do > make the code use hardcoded tabs, but I think it would be worth > mentioning that in the commit message. (I assumed from the wording > -- and also because my mail client was being misleading -- that > these changes were adding 8-space indent.) It was a mixture of problems. Some places where using 4 spaces instead of 1 tab, other places used a mixture of tabs and 4 spaces and one place used only tabs, but too few of them. > > Otherwise > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > thanks > -- PMM > With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2025 Red Hat, Inc.