rhkdump / kdump-utils

Kernel crash dump collection utilities
GNU General Public License v2.0
3 stars 12 forks source link

Use "grep -q <<< $(cmd)" instead of "cmd | grep -q" #17

Closed liutgnu closed 3 months ago

liutgnu commented 3 months ago
Use "grep -q <<< $(cmd)" instead of "cmd | grep -q"

The following line of code's exit value should be the exit value of
grep by default:

    cmd | grep -q

However it will not always work as expected, a 141 exit code may be
returned, see the following debug info:

--- a/usr/lib/dracut/modules.d/99kdumpbase/module-setup.sh
+++ b/usr/lib/dracut/modules.d/99kdumpbase/module-setup.sh
@@ -55,6 +55,11 @@ depends() {
         _dep="$_dep ssh-client"
     fi

+    dracut --list-modules 1>&2
+    echo $? 1>&2
+    dracut --list-modules | grep -q lvmthinpool-monitor
+    echo $? ${PIPESTATUS[0]} ${PIPESTATUS[1]} 1>&2
+
     if is_lvm2_thinp_dump_target; then
         if dracut --list-modules | grep -q lvmthinpool-monitor; then
             add_opt_module lvmthinpool-monitor

$ kdumpctl rebuild
kdump: Rebuilding /boot/initramfs-6.10.0-0.rc4.11.el10.x86_64kdump.img
...snip...
lvmmerge
lvmthinpool-monitor
..snip...
uefi-lib
0
141 141 0

The reason is, grep exits immediately when there is a match but the
first cmd still keep writing to the pipe. Since there is no reader,
a SIGPIPE signal is generated. As a result, a 141 exit code will be
returned.

Let's use the following line of code instead, which have the same effect
but works as expected:

    grep -q <<< $(cmd)

Signed-off-by: Tao Liu <ltao@redhat.com>
coiby commented 3 months ago

Out of curiosity, I did a further look. According to this answer, this is because grep exits immediately when there is a match but the first cmd still keep writing to the pipe. Since there is no reader, a SIGPIPE signal is generated. This 141 exit code issue can be reproduced in the following way,

[root@localhost test]# seq 1 10000 | head -1
1
[root@localhost test]# echo $?
141

Can you include the above info the commit message? Btw, you may use "```" to wrap the debug info to have a better visual effect.

liutgnu commented 3 months ago

Hi Coiby,

Thanks a lot for finding out the root cause! It confused me for a long time. And I will update the commit log.

Thanks, Tao Liu

liutgnu commented 3 months ago

Hello @coiby , commit log updated, please have a review, thanks!