aboutsummaryrefslogtreecommitdiffstats
path: root/erts/emulator/drivers
diff options
context:
space:
mode:
authorMikael Pettersson <[email protected]>2015-12-20 17:02:15 +0100
committerMikael Pettersson <[email protected]>2015-12-20 18:05:51 +0100
commitfe72df791e7857fa72f6ac2b7ba476212a0c2edd (patch)
tree71d50c5f917a9316cb7442e0b6aad46690b49d6b /erts/emulator/drivers
parent21d6192389a04024f7a41ced9d0911a9cce6f4e8 (diff)
downloadotp-fe72df791e7857fa72f6ac2b7ba476212a0c2edd.tar.gz
otp-fe72df791e7857fa72f6ac2b7ba476212a0c2edd.tar.bz2
otp-fe72df791e7857fa72f6ac2b7ba476212a0c2edd.zip
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).
Diffstat (limited to 'erts/emulator/drivers')
-rw-r--r--erts/emulator/drivers/common/efile_drv.c8
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;