From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicholas Piggin Date: Sat, 06 Apr 2024 16:31:17 +1000 Subject: [kvm-unit-tests RFC PATCH 17/17] shellcheck: Suppress various messages In-Reply-To: <20240405-7c0ad5d3ce76e1ad9ad2f5a9@orel> References: <20240405090052.375599-1-npiggin@gmail.com> <20240405090052.375599-18-npiggin@gmail.com> <20240405-7c0ad5d3ce76e1ad9ad2f5a9@orel> Message-ID: List-Id: To: kvm-riscv@lists.infradead.org MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Sat Apr 6, 2024 at 12:55 AM AEST, Andrew Jones wrote: > On Fri, Apr 05, 2024 at 07:00:49PM +1000, Nicholas Piggin wrote: > > Various info and warnings are suppressed here, where circumstances > > (commented) warrant. > > > > Signed-off-by: Nicholas Piggin > > --- > > run_tests.sh | 3 +++ > > scripts/arch-run.bash | 9 +++++++++ > > scripts/mkstandalone.sh | 2 ++ > > scripts/runtime.bash | 2 ++ > > 4 files changed, 16 insertions(+) > > > > diff --git a/run_tests.sh b/run_tests.sh > > index 938bb8edf..152323ffc 100755 > > --- a/run_tests.sh > > +++ b/run_tests.sh > > @@ -45,6 +45,9 @@ fi > > only_tests="" > > list_tests="" > > args=$(getopt -u -o ag:htj:vl -l all,group:,help,tap13,parallel:,verbose,list,probe-maxsmp -- "$@") > > +# Shellcheck likes to test commands directly rather than with $? but sometimes they > > +# are too long to put in the same test. > > +# shellcheck disable=SC2181 > > [ $? -ne 0 ] && exit 2; > > set -- $args; > > while [ $# -gt 0 ]; do > > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash > > index ed440b4aa..fe8785cfd 100644 > > --- a/scripts/arch-run.bash > > +++ b/scripts/arch-run.bash > > @@ -44,6 +44,8 @@ run_qemu () > > if [ "$errors" ]; then > > sig=$(grep 'terminating on signal' <<<"$errors") > > if [ "$sig" ]; then > > + # This is too complex for ${var/search/replace} > > + # shellcheck disable=SC2001 > > sig=$(sed 's/.*terminating on signal \([0-9][0-9]*\).*/\1/' <<<"$sig") > > fi > > fi > > @@ -174,9 +176,12 @@ run_migration () > > > > # Holding both ends of the input fifo open prevents opens from > > # blocking and readers getting EOF when a writer closes it. > > + # These fds appear to be unused to shellcheck so quieten the warning. > > mkfifo ${src_infifo} > > mkfifo ${dst_infifo} > > + # shellcheck disable=SC2034 > > exec {src_infifo_fd}<>${src_infifo} > > + # shellcheck disable=SC2034 > > exec {dst_infifo_fd}<>${dst_infifo} > > > > "${migcmdline[@]}" \ > > @@ -184,6 +189,8 @@ run_migration () > > -mon chardev=mon,mode=control \ > > < ${src_infifo} > ${src_outfifo} & > > live_pid=$! > > + # SC complains about useless cat but I prefer it over redirect here. > > Let's spell out 'shellcheck' when referring to it rather than call it > 'SC'. And instead of "but I prefer..." let's write > > # shellcheck complains about a useless cat, but using a redirect here is > # harder to read Sounds good, will do. > > or something like that. Don't tell my cat-loving daughter that I just > wrote "a useless cat"! :D > > > > + # shellcheck disable=SC2002 > > cat ${src_outfifo} | tee ${src_out} | filter_quiet_msgs & > > > > # Start the first destination QEMU machine in advance of the test > > @@ -224,6 +231,8 @@ do_migration () > > -mon chardev=mon,mode=control -incoming unix:${dst_incoming} \ > > < ${dst_infifo} > ${dst_outfifo} & > > incoming_pid=$! > > + # SC complains about useless cat but I prefer it over redirect here. > > Same comment as above. > > > + # shellcheck disable=SC2002 > > cat ${dst_outfifo} | tee ${dst_out} | filter_quiet_msgs & > > > > # The test must prompt the user to migrate, so wait for the > > diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh > > index 756647f29..2318a85f0 100755 > > --- a/scripts/mkstandalone.sh > > +++ b/scripts/mkstandalone.sh > > @@ -65,6 +65,8 @@ generate_test () > > fi > > > > temp_file bin "$kernel" > > + # Don't want to expand $bin but print it as-is. > > + # shellcheck disable=SC2016 > > args[3]='$bin' > > > > (echo "#!/usr/bin/env bash" > > diff --git a/scripts/runtime.bash b/scripts/runtime.bash > > index 3b76aec9e..c87613b96 100644 > > --- a/scripts/runtime.bash > > +++ b/scripts/runtime.bash > > @@ -137,6 +137,8 @@ function run() > > # the check line can contain multiple files to check separated by a space > > # but each check parameter needs to be of the form = > > if [ "$check" ]; then > > + # There is no globbing allowed in the check parameter. > > + # shellcheck disable=SC2206 > > check=($check) > > Hmm, I'm not sure about this one. $check is an arbitrary path, which means > it can have spaces, then =, and then an arbitrary value, which means it can > contain spaces. If there are multiple check path=value pairs then > separation by space is a bad idea, and any deliminator really is. It seems > like each pair should be quoted, i.e. > > check = "path1=value1" "path2=value2" > > and then that should be managed here. Yeah I did think of that, valid paths could also have = and ", and even with double quotes it seems to be tricky to handle spaces. Maybe I'll just add to the unittest.cfg docs to stick with alphanumeric paths, and we can improve it if someone complains? Thanks, Nick From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f179.google.com (mail-pf1-f179.google.com [209.85.210.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 26FFFFC19 for ; Sat, 6 Apr 2024 06:31:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.179 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712385090; cv=none; b=vB65Cm6jdFuE61LXM5g6ByQdNuuYPs1phZyPxa8mbJcOJfo5z2/+OkxYdBLpAMRnClz0RAGDvepM7IsuCgw7/RdUys+jXwXGj1/gxt8oExSfwF7EEEvR08XOvHyAtWP8aj+d5yD44c/MQKVofUQYKKZGQ7uzY23d2sjlclf57lw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712385090; c=relaxed/simple; bh=KV/wB10K9PldvqN8/z4eMzHUcgIzx9r1HmzzSRp81s8=; h=Mime-Version:Content-Type:Date:Message-Id:Subject:From:To:Cc: References:In-Reply-To; b=o9Mayb8m4xsFNjnX4L3LyQ4NIsa7hjsjjy4R1luuOjmQR6rtKZLsNBz0LyAzqTX05FRVv5pbNOXkZ2lqkcu1xJpm/KEAvIKGsh416IeIsM5bgtubbLgt2XiRPriVBCP8lb8nc3TON5ppDdKcNIPKfl5gQYPcJ6fDekUdw1Jeh04= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=YcV69PR4; arc=none smtp.client-ip=209.85.210.179 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="YcV69PR4" Received: by mail-pf1-f179.google.com with SMTP id d2e1a72fcca58-6ed11782727so215122b3a.1 for ; Fri, 05 Apr 2024 23:31:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1712385088; x=1712989888; darn=lists.linux.dev; h=in-reply-to:references:cc:to:from:subject:message-id:date :content-transfer-encoding:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=C3gFO0LBD4TsLCWewzw78bLLIgfF1Yq2lATDwkzzjxE=; b=YcV69PR4JQY4xbndPaL7F60qHxRN92q1duoUoZvYYUHxlYLgNpGhCrmQP9Qzqf3zm9 4CS4MIOUycDXm0BplUPamOV73a6gpTCdjV65T2NMu6tCL0VQN4UxeYNp1lRRTeO7Y8cj 04BSU3nfk81bdFcaTBz1bKYuDE0Cx0KXSU7c++AE88/vW7rnMQLsYDmguceTZlL5MKx4 HLJBJje1VYg4oBaaMXps39LoKztEORiWgpMqBZFbQQ573HL9CuuZ2/2I+ymROnFQqyUX pqpuLpQG2EwR97zMdN/LRdVDHovKp+8z8cod+Qfd19rYRe0ULUVPLPLYAgIHFxUhjzJ1 Wq9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712385088; x=1712989888; h=in-reply-to:references:cc:to:from:subject:message-id:date :content-transfer-encoding:mime-version:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=C3gFO0LBD4TsLCWewzw78bLLIgfF1Yq2lATDwkzzjxE=; b=DHc27/v2sS2QnROnkCf0LXI853EpQU/qV5UOAh/8yS7GMo4CVMOgnFke1FERUwG9xV dJ5AKZOIk6iO6m7a+et60CU433FoJuZ8vL/iG6X+8V8i1nFGxa6cA5TeNjgpibqLwOOV QI1dT5/0/fNKefvHIQ9yJAAQEOHb0ZVsySNnd7YwlM966Xd6owK4XSEEk0dHyUn8QpSJ oOGiE/b+sV3cy10btgEZqmnygxhrY6DI5SnbwGQYVy7UMLguVk5lPC9Sxe89wEtxczVp fdUGKUEKmZzoOW2x1954OijRTckbWpQZdvTS0kV5fT3mw1nbyDe9BWCDw9nAmL80git3 zHRA== X-Forwarded-Encrypted: i=1; AJvYcCXh8dlEO0oE3E3KnvXkb1ovrwPsyYWrZ/LHxLGZAG5BFWpD+2IYXcsVrmW1RpNLvezLoT3O8mUL2uFc874ZrB8f2Rn51ww5 X-Gm-Message-State: AOJu0Yzz+ROR2MoiMAZESeq3eGjb/X2K2UmUaHN6EurkqFASw8hHjnpK hl1ukt+GE8J9zMOHyjMMObew20rpZSj++dRo3ZzunFI34vBXBHN/ X-Google-Smtp-Source: AGHT+IFALmVk08NKzMWMyCDJd6QtuwmTTglOMhS/paEBbUp86/OqFJdFQ+4zrRK30sQPpbpFcknh1Q== X-Received: by 2002:aa7:8893:0:b0:6ec:ed90:65ea with SMTP id z19-20020aa78893000000b006eced9065eamr3711769pfe.32.1712385088140; Fri, 05 Apr 2024 23:31:28 -0700 (PDT) Received: from localhost (124-169-104-130.tpgi.com.au. [124.169.104.130]) by smtp.gmail.com with ESMTPSA id km23-20020a056a003c5700b006e6bf17ba8asm2549066pfb.65.2024.04.05.23.31.19 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 05 Apr 2024 23:31:27 -0700 (PDT) Precedence: bulk X-Mailing-List: kvmarm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Sat, 06 Apr 2024 16:31:17 +1000 Message-Id: Subject: Re: [kvm-unit-tests RFC PATCH 17/17] shellcheck: Suppress various messages From: "Nicholas Piggin" To: "Andrew Jones" Cc: "Paolo Bonzini" , "Thomas Huth" , "Alexandru Elisei" , "Eric Auger" , "Janosch Frank" , "Claudio Imbrenda" , =?utf-8?q?Nico_B=C3=B6hr?= , "David Hildenbrand" , "Shaoqin Huang" , "Nikos Nikoleris" , "Nadav Amit" , "David Woodhouse" , "Ricardo Koller" , "rminmin" , "Gavin Shan" , "Nina Schoetterl-Glausch" , "Sean Christopherson" , , , , X-Mailer: aerc 0.17.0 References: <20240405090052.375599-1-npiggin@gmail.com> <20240405090052.375599-18-npiggin@gmail.com> <20240405-7c0ad5d3ce76e1ad9ad2f5a9@orel> In-Reply-To: <20240405-7c0ad5d3ce76e1ad9ad2f5a9@orel> On Sat Apr 6, 2024 at 12:55 AM AEST, Andrew Jones wrote: > On Fri, Apr 05, 2024 at 07:00:49PM +1000, Nicholas Piggin wrote: > > Various info and warnings are suppressed here, where circumstances > > (commented) warrant. > >=20 > > Signed-off-by: Nicholas Piggin > > --- > > run_tests.sh | 3 +++ > > scripts/arch-run.bash | 9 +++++++++ > > scripts/mkstandalone.sh | 2 ++ > > scripts/runtime.bash | 2 ++ > > 4 files changed, 16 insertions(+) > >=20 > > diff --git a/run_tests.sh b/run_tests.sh > > index 938bb8edf..152323ffc 100755 > > --- a/run_tests.sh > > +++ b/run_tests.sh > > @@ -45,6 +45,9 @@ fi > > only_tests=3D"" > > list_tests=3D"" > > args=3D$(getopt -u -o ag:htj:vl -l all,group:,help,tap13,parallel:,ver= bose,list,probe-maxsmp -- "$@") > > +# Shellcheck likes to test commands directly rather than with $? but s= ometimes they > > +# are too long to put in the same test. > > +# shellcheck disable=3DSC2181 > > [ $? -ne 0 ] && exit 2; > > set -- $args; > > while [ $# -gt 0 ]; do > > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash > > index ed440b4aa..fe8785cfd 100644 > > --- a/scripts/arch-run.bash > > +++ b/scripts/arch-run.bash > > @@ -44,6 +44,8 @@ run_qemu () > > if [ "$errors" ]; then > > sig=3D$(grep 'terminating on signal' <<<"$errors") > > if [ "$sig" ]; then > > + # This is too complex for ${var/search/replace} > > + # shellcheck disable=3DSC2001 > > sig=3D$(sed 's/.*terminating on signal \([0-9][0-9]*\).*/\1/' <<<"$= sig") > > fi > > fi > > @@ -174,9 +176,12 @@ run_migration () > > =20 > > # Holding both ends of the input fifo open prevents opens from > > # blocking and readers getting EOF when a writer closes it. > > + # These fds appear to be unused to shellcheck so quieten the warning. > > mkfifo ${src_infifo} > > mkfifo ${dst_infifo} > > + # shellcheck disable=3DSC2034 > > exec {src_infifo_fd}<>${src_infifo} > > + # shellcheck disable=3DSC2034 > > exec {dst_infifo_fd}<>${dst_infifo} > > =20 > > "${migcmdline[@]}" \ > > @@ -184,6 +189,8 @@ run_migration () > > -mon chardev=3Dmon,mode=3Dcontrol \ > > < ${src_infifo} > ${src_outfifo} & > > live_pid=3D$! > > + # SC complains about useless cat but I prefer it over redirect here. > > Let's spell out 'shellcheck' when referring to it rather than call it > 'SC'. And instead of "but I prefer..." let's write > > # shellcheck complains about a useless cat, but using a redirect here is > # harder to read Sounds good, will do. > > or something like that. Don't tell my cat-loving daughter that I just > wrote "a useless cat"! :D > > > > + # shellcheck disable=3DSC2002 > > cat ${src_outfifo} | tee ${src_out} | filter_quiet_msgs & > > =20 > > # Start the first destination QEMU machine in advance of the test > > @@ -224,6 +231,8 @@ do_migration () > > -mon chardev=3Dmon,mode=3Dcontrol -incoming unix:${dst_incoming} \ > > < ${dst_infifo} > ${dst_outfifo} & > > incoming_pid=3D$! > > + # SC complains about useless cat but I prefer it over redirect here. > > Same comment as above. > > > + # shellcheck disable=3DSC2002 > > cat ${dst_outfifo} | tee ${dst_out} | filter_quiet_msgs & > > =20 > > # The test must prompt the user to migrate, so wait for the > > diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh > > index 756647f29..2318a85f0 100755 > > --- a/scripts/mkstandalone.sh > > +++ b/scripts/mkstandalone.sh > > @@ -65,6 +65,8 @@ generate_test () > > fi > > =20 > > temp_file bin "$kernel" > > + # Don't want to expand $bin but print it as-is. > > + # shellcheck disable=3DSC2016 > > args[3]=3D'$bin' > > =20 > > (echo "#!/usr/bin/env bash" > > diff --git a/scripts/runtime.bash b/scripts/runtime.bash > > index 3b76aec9e..c87613b96 100644 > > --- a/scripts/runtime.bash > > +++ b/scripts/runtime.bash > > @@ -137,6 +137,8 @@ function run() > > # the check line can contain multiple files to check separated by = a space > > # but each check parameter needs to be of the form =3D > > if [ "$check" ]; then > > + # There is no globbing allowed in the check parameter. > > + # shellcheck disable=3DSC2206 > > check=3D($check) > > Hmm, I'm not sure about this one. $check is an arbitrary path, which mean= s > it can have spaces, then =3D, and then an arbitrary value, which means it= can > contain spaces. If there are multiple check path=3Dvalue pairs then > separation by space is a bad idea, and any deliminator really is. It seem= s > like each pair should be quoted, i.e. > > check =3D "path1=3Dvalue1" "path2=3Dvalue2" > > and then that should be managed here. Yeah I did think of that, valid paths could also have =3D and ", and even with double quotes it seems to be tricky to handle spaces. Maybe I'll just add to the unittest.cfg docs to stick with alphanumeric paths, and we can improve it if someone complains? Thanks, Nick