----- Original Message -----
From: "Pratyush Anand" <panand(a)redhat.com>
To: "Pingfan Liu" <piliu(a)redhat.com>, kexec(a)lists.fedoraproject.org
Cc: "Dave Young" <dyoung(a)redhat.com>, "Xunlei Pang"
<xpang(a)redhat.com>, "Baoquan He" <bhe(a)redhat.com>
Sent: Wednesday, February 8, 2017 1:33:12 PM
Subject: Re: [f25 PATCH 1/4] kdump-lib.sh: fix incorrect usage with pipe as input for
grep -q
On Tuesday 07 February 2017 11:59 AM, Pingfan Liu wrote:
> For -q option, as man grep says: Exit immediately with zero status if
> any match is found, even if an error was detected.
> So when matching, the read side of pipe is closed by "grep -q", while
> the write side still try to write more data, which cause SIGPIPE to the
> process, and the shell can not exit with 0.
>
While it seems from the above logic that there might be some corner
cases where we will have SIGPIPE, but do we really need to replace it
everywhere. `grep -q` could be faster than `grep`.
Yeah. But its behavior relies on the kernel implement of pipe. I think >4KB can
cause the problem
Thx,
Pingfan
> > Signed-off-by: Pingfan Liu <piliu(a)redhat.com>
>
> Anyway, as Dave suggested, an example of failure case could be good.
>
> Otherwise, patches looks fine.
>
> ~Pratyush
> > ---
> > kdump-lib.sh | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/kdump-lib.sh b/kdump-lib.sh
> > index e3b5a01..21e05f1 100755
> > --- a/kdump-lib.sh
> > +++ b/kdump-lib.sh
> > @@ -63,7 +63,7 @@ is_pcs_fence_kdump()
> > [ -x $FENCE_KDUMP_SEND ] || return 1
> >
> > # fence kdump not configured?
> > - (pcs cluster cib | grep -q 'type="fence_kdump"')
&> /dev/null ||
> > return 1
> > + (pcs cluster cib | grep 'type="fence_kdump"') &>
/dev/null || return 1
> > }
> >
> > # Check if fence_kdump is configured using kdump options
> > @@ -233,7 +233,7 @@ is_atomic()
> >
> > is_ipv6_address()
> > {
> > - echo $1 | grep -q ":"
> > + (echo $1 | grep ":") &> /dev/null
> > }
> >
> > # get ip address or hostname from nfs/ssh config value
> > @@ -257,7 +257,7 @@ is_hostname()
> > if [ -n "$_hostname" ]; then
> > return 1
> > fi
> > - echo $1 | grep -q "[a-zA-Z]"
> > + (echo $1 | grep "[a-zA-Z]") &> /dev/null
> > }
> >
> > # Copied from "/etc/sysconfig/network-scripts/network-functions"
> > @@ -303,8 +303,8 @@ is_nm_running()
> >
> > is_nm_handling()
> > {
> > - LANG=C nmcli -t --fields device,state dev status 2>/dev/null \
> > - | grep -q "^\(${1}:connected\)\|\(${1}:connecting.*\)$"
> > + (LANG=C nmcli -t --fields device,state dev status \
> > + | grep "^\(${1}:connected\)\|\(${1}:connecting.*\)$")
&>
> > /dev/null
> > }
> >
> > # $1: netdev name
> > @@ -377,7 +377,7 @@ is_wdt_mod_omitted() {
> > [[ -z $1 ]] && break
> > case $1 in
> > -o|--omit)
> > - echo $2 | grep -qw "watchdog"
> > + (echo $2 | grep -w "watchdog") &> /dev/null
> > [[ $? == 0 ]] && ret=0
> > break
> > esac
> >
>