[Bug 190793] Some rc scripts return non zero status on success

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=190793

Devin Teske <dteske at FreeBSD.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dteske at FreeBSD.org

--- Comment #6 from Devin Teske <dteske at FreeBSD.org> ---
The attached patch upon review should NOT be applied. There is a better way
that produces smaller diff while maintaining logical operands.

The principal reason why the following can produce error status is because the
wrong logical operand is used with the wrong condition:

[ -n "${foo}" ] && echo '.'

We will call the following the "l-value" to the logical AND operator (&&):

[ -n "${foo}" ]

and we will call the following the "r-value" to '&&' operator:

echo '.'

The r-value is only evaluated when the l-value is true, and thus when the
l-value returns false the logical AND result is likewise false (error status).

If you flip the conditional on its head and use the opposite logical operator
(the OR operator in this case; "||"), then the l-value leaves a desirable
success-status and desirably the r-value is not executed when the inverted
condition is true. That sounds like a headache, but it's really simple. The
above translates to:

[ -z "${foo}" ] || echo '.'

Here we say that if ${foo} expands to an empty string (unset or NULL), we don't
execute the r-value.

The net-effect is that "||" is almost always what you want when you are
concerned about exit status.

However, you have to be very careful in the rc.d world when you flip an
"l-value && r-value" into an "! l-value || r-value" because of the possibility
that "set -e" may become neutered for a particular condition.

The "set -e" directive in shell makes all errors fatal. The below will trigger
a premature termination when "set -e" is in-effect (because the logical AND is
only successful if both l-value and r-value return success, a failure by either
l-value or r-value will result in premature termination):

[ -n "${foo}" ] && echo '.'

Meanwhile, flipping this on it's head, despite appearing to not have an effect
code-wise, would negate a premature termination should "set -e" be in-effect:

[ -z "${foo}" ] || echo '.'

That's because the result of a logical OR between two commands is going to be
success if either the l-value or r-value returns successfully.

Therefore, the way to address the reported problem of unassailable return
status is not to blindly translate all logical AND operations into
if-statements as the original patch would have done, nor do we blindly
translate to logical OR operations, but really we need to look at each use-case
of logical AND and/or OR and determine:

a. Should premature termination occur for either l-value or r-value in the
event of a "set -e"
b. Is the logical operation at the end of a function or script

If neither of those things are true, leave it alone if it accommodates (a)
above correctly as-intended.

-- 
You are receiving this mail because:
You are the assignee for the bug.