fstests.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Zorro Lang <zlang@redhat.com>
To: Murphy Zhou <xzhou@redhat.com>
Cc: fstests@vger.kernel.org, Luis Chamberlain <mcgrof@kernel.org>
Subject: Re: [PATCH] common/rc: fix MKSWAP_PROG quoting
Date: Tue, 31 Aug 2021 13:26:32 +0800	[thread overview]
Message-ID: <20210831052632.p4kaygjeujipo4w5@fedora> (raw)
In-Reply-To: <20210831030426.nwcoelcnshq4wrxx@xzhoux.usersys.redhat.com>

On Tue, Aug 31, 2021 at 11:04:26AM +0800, Murphy Zhou wrote:
> After commit
>   0e4dd8b9 common/rc: fix ignoring of errors on
> we are getting this error message when running swapfiles tests:
>   +./common/rc: line 2553: MKSWAP_PROG: command not found
> 
> Signed-off-by: Murphy Zhou <xzhou@redhat.com>
> ---
>  common/rc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/rc b/common/rc
> index 46b6b220..0597de13 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2550,7 +2550,7 @@ _format_swapfile() {
>  	$CHATTR_PROG +C "$fname" > /dev/null 2>&1
>  	_pwrite_byte 0x61 0 "$sz" "$fname" >> $seqres.full
>  	# Ignore permission complaints on filesystems that don't support perms
> -	$(MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2)) >> $seqres.full
> +	($MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2)) >> $seqres.full

The change history is:

Commit 0c95c8ac tried to "hide permision warning", so did:
-       $MKSWAP_PROG "$fname" >> $seqres.full
+       # Ignore permission complaints on filesystems that don't support perms
+       $MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2) >> $seqres.full

Then commit 0e4dd8b9 said "it broke older versions of bash such as 4.4.23", so tried
to use "a $(foo) to run the executable":
-       $MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2) >> $seqres.full
+       $(MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2)) >> $seqres.full

Now this patch try to do ($FOO_PROG):
($MKSWAP_PROG "$fname" .....)

I'm *not* a bash expert, if I'm wrong feel free to correct me:) If the original problem
is trying to hide permision warning from stderr, and to avoid new syntax breaking old
bash, Why we must struggle with this complex syntax which isn't compatible. How about:

$MKSWAP_PROG "$fname" >> $seqres.full 2>&1 | grep -v "insecure permission"

Or other better and compatible way ?

Thanks,
Zorro

>  }
>  
>  _swapon_file() {
> -- 
> 2.20.1
> 


  reply	other threads:[~2021-08-31  5:06 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-31  3:04 [PATCH] common/rc: fix MKSWAP_PROG quoting Murphy Zhou
2021-08-31  5:26 ` Zorro Lang [this message]
2021-08-31  7:38   ` Pavel Reichl
2021-08-31 13:42     ` Zorro Lang
2021-08-31 17:05       ` Theodore Ts'o
2021-09-01  6:16         ` [PATCH v2] " Murphy Zhou
2021-09-01  7:15           ` Dave Chinner
2021-09-01  9:40             ` [PATCH v3] " Xiong Zhou
2021-09-03 17:48               ` Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210831052632.p4kaygjeujipo4w5@fedora \
    --to=zlang@redhat.com \
    --cc=fstests@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=xzhou@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).