efile_drv: logic error in compressed file write

Compiling OTP 18.2.1 with gcc-5.3 shows the following warning:

drivers/common/efile_drv.c:1538:23: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]

The code in question is:

        if (! (status =
               erts_gzwrite((ErtsGzFile)d->fd,
                            iov[i].iov_base,
                            iov[i].iov_len)) == iov[i].iov_len) {
            d->errInfo.posix_errno =
                d->errInfo.os_errno = errno; /* XXX Correct? */
            break;
        }

If we hoist the assignment out of the if for clarity, it becomes:

        status = erts_gzwrite(..., iov[i].iov_len);
        if (! status == iov[i].iov_len) { ...; break; }

iov_len is > 0 here, and status will equal iov_len if erts_gzwrite
succeeded, but will be less than iov_len if an error occurred.

"! status" is 0 or 1, which can only equal iov_len if iov_len is 1 and
erts_gzwrite detected an error and returned 0.

The effect of this mistake is that any error when iov_len >= 2 will
skip the conditional code and break statement.  In particular, partial
writes (0 < status && status < iov_len) will not be flagged as errors.

All releases since OTP R8B-0 are affected.

The variable "status" is really a boolean, which is to be set to zero
on error.  The fix is to set status to 1 if erts_gzwrite() returned iov_len
and 0 otherwise, and to change the condition to "if (! status) ...".

I'm also hoisting the assignment out of the condition since it obscures
the code while providing not benefit (the condition in a while or for
loop would be a different matter).

1 files changed, 4 insertions, 4 deletions

diff --git a/erts/emulator/drivers/common/efile_drv.c b/erts/emulator/drivers/common/efile_drv.c
index 3b6abec25e..a5a5dfb7f8 100644
--- a/erts/emulator/drivers/common/efile_drv.c
+++ b/erts/emulator/drivers/common/efile_drv.c
@@ -1532,10 +1532,10 @@ static void invoke_writev(void *data) {
 		     * with errno.
 		     */
 		    errno = EINVAL; 
-		    if (! (status = 
-			   erts_gzwrite((ErtsGzFile)d->fd,
-					iov[i].iov_base,
-					iov[i].iov_len)) == iov[i].iov_len) {
+		    status = erts_gzwrite((ErtsGzFile)d->fd,
+					  iov[i].iov_base,
+					  iov[i].iov_len) == iov[i].iov_len;
+		    if (! status) {
 			d->errInfo.posix_errno =
 			    d->errInfo.os_errno = errno; /* XXX Correct? */
 			break;