Hi Coiby,
On Fri, 25 Feb 2022 14:45:27 +0800
Coiby Xu <coxu(a)redhat.com> wrote:
On Thu, Feb 24, 2022 at 05:49:33PM +0100, Philipp Rudo wrote:
>Hi Coiby,
>
>> Thanks for correcting my misunderstanding! But why not simply removing
>> "return $?" instead?
>
>I don't fully understand your question. Do you mean doing something like
>this?
Sorry for not being clear enough.
>
>diff --git a/kdumpctl b/kdumpctl
>index 630e56f..24c266f 100755
>--- a/kdumpctl
>+++ b/kdumpctl
>@@ -1121,11 +1121,10 @@ reload_fadump()
> # to handle such scenario.
> if stop_fadump; then
> start_fadump
>- return
Although I only suggested removing this return originally (which obvious
is incorrect if I read the remaining code in reload_fadump),
> fi
> fi
>
>- return 1
>+ return
actually it works with this return also removed.
hmm... you are right that this 'return' is redundant as functions, like
'if' statements, automatically return the value of the last command
executed when they reach the end. However, dropping it still changes the
behavior compared to the original version.
> }
>
> stop()
>
>Unfortunately that won't work as expected.
> The problem is that in bash
>control statements like 'if' or 'for' do have a return value. This
is
>usually the return value of the last command executed _or_ 0 if, e.g. no
>test condition is true for 'if' (for a full list see 'Compound
Commands'
>section in the bash man page). Thus the above will change the return
>value in the case that stop_fadump fails.
>
>You can test that behavior with this snippet
>
>if false; then
> false
>fi && echo "return value is 0" || echo "return value is
1"
>
>which will print "return value is 0". However, when you update
>'if false;' to 'if true;' then it will print "return value is
1".
If we modify your example to the same if/else/fi structure as
reload_fadump,
if false; then
false
else
true
fi && echo "return value is 0" || echo "return value is 1"
this will print "return value is 0". And changing the only true to
false will print "return value is 1".
That's absolutely right. But you're still missing that the original
code returned 'false' in that case. So dropping the return will change
the return value of the function for that case.
Let's look at the full function. For the problematic case the original
code looked like this
f() {
if false; then
echo "outer if"
return
else
if false; then
echo "inner if"
return
fi
fi
return 1
}
For this case the return value of the function will always be 1 (aka.
false).
If we changed the last line of the function to a simple 'return' (or
drop the line altogether), then the return value will be determined by
the outer 'if' statement. For that case it will be the return value of
the last command executed in the 'else' block. But the 'else' block
only has one command which is the inner 'if' statement. For the inner
'if' none of the conditions are true so it returns 0 (aka. true). So
the return value of the function changed from 1 to 0.
Thanks
Philipp