Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

liutgnu
Copy link
Collaborator

@liutgnu liutgnu commented Jun 25, 2024

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>

@liutgnu liutgnu requested a review from coiby June 25, 2024 05:18
@coiby
Copy link
Member

coiby commented Jun 26, 2024

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
Copy link
Collaborator Author

liutgnu commented Jun 26, 2024

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

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>
@liutgnu
Copy link
Collaborator Author

liutgnu commented Jun 27, 2024

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

@coiby coiby merged commit 98087d7 into rhkdump:main Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants