diff options
author | Andrew Clayton <a.clayton@nginx.com> | 2023-03-24 14:23:06 +0000 |
---|---|---|
committer | Andrew Clayton <a.clayton@nginx.com> | 2023-04-24 19:40:16 +0100 |
commit | 375556f9aa76c1b9ff77d08f75451dfffb1e082a (patch) | |
tree | 899745a3ce8c88ec1ccb450a7c7c3ca1ed394b6a /src | |
parent | b9177d36e71a9f62198b00fa40f277c06d2264bb (diff) | |
download | unit-375556f9aa76c1b9ff77d08f75451dfffb1e082a.tar.gz unit-375556f9aa76c1b9ff77d08f75451dfffb1e082a.tar.bz2 |
Don't conflate the error variable in nxt_kqueue_poll().
In nxt_kqueue_poll() error is declared as a nxt_bool_t aka unsigned int
(on x86-64 anyway).
It is used both as a boolean and as the return storage for a bitwise AND
operation.
This has potential to go awry.
If nxt_bool_t was changed to be a u8 then we would have the following
issue
gcc12 -c -pipe -fPIC -fvisibility=hidden -O -W -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wmissing-prototypes -Werror -g -O2 -I src -I build -I/usr/local/include -o build/src/nxt_kqueue_engine.o -MMD -MF build/src/nxt_kqueue_engine.dep -MT build/src/nxt_kqueue_engine.o src/nxt_kqueue_engine.c
src/nxt_kqueue_engine.c: In function 'nxt_kqueue_poll':
src/nxt_kqueue_engine.c:728:17: error: overflow in conversion from 'int' to 'nxt_bool_t' {aka 'unsigned char'} changes value from '(int)kev->flags & 16384' to '0' [-Werror=overflow]
728 | error = (kev->flags & EV_ERROR);
| ^
cc1: all warnings being treated as errors
EV_ERROR has the value 16384, after the AND operation error holds 16384,
however this overflows and wraps around (64 times) exactly to 0.
With nxt_bool_t defined as a u32, we would have a similar issue if
EV_ERROR ever became UINT_MAX + 1 (or a multiple thereof)...
Rather than conflating the use of error, keep error as a boolean (it is
used further down the function) but do the AND operation inside the
if ().
Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
Diffstat (limited to 'src')
-rw-r--r-- | src/nxt_kqueue_engine.c | 7 |
1 files changed, 4 insertions, 3 deletions
diff --git a/src/nxt_kqueue_engine.c b/src/nxt_kqueue_engine.c index ecc3251e..a7a5a29e 100644 --- a/src/nxt_kqueue_engine.c +++ b/src/nxt_kqueue_engine.c @@ -716,6 +716,8 @@ nxt_kqueue_poll(nxt_event_engine_t *engine, nxt_msec_t timeout) for (i = 0; i < nevents; i++) { + error = 0; + kev = &engine->u.kqueue.events[i]; nxt_debug(&engine->task, @@ -725,12 +727,11 @@ nxt_kqueue_poll(nxt_event_engine_t *engine, nxt_msec_t timeout) kev->ident, kev->filter, kev->flags, kev->fflags, kev->data, kev->udata); - error = (kev->flags & EV_ERROR); - - if (nxt_slow_path(error)) { + if (nxt_slow_path(kev->flags & EV_ERROR)) { nxt_alert(&engine->task, "kevent(%d) error %E on ident:%d filter:%d", engine->u.kqueue.fd, kev->data, kev->ident, kev->filter); + error = 1; } task = &engine->task; |