From 5cb56389bdf78f98e44128afca39fba2dac74dfa Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Tue, 30 Oct 2018 18:09:12 +0000 Subject: [PATCH] Remove three uses of bitwise ops on boolean values. If values are boolean, it's confusing to use & and | in place of && and ||. In two of these three cases it was simply a typo and I've used the other one; in the third, it was a deliberate avoidance of short- circuit evaluation (and commented as such), but having seen how easy it is to make the same typo by accident, I've decided it's clearer to just move the LHS and RHS evaluations outside the expression. --- mainchan.c | 2 +- ssh2transport.c | 10 +++++----- unix/gtkask.c | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/mainchan.c b/mainchan.c index a75b44e4..c0738151 100644 --- a/mainchan.c +++ b/mainchan.c @@ -376,7 +376,7 @@ static void mainchan_send_eof(Channel *chan) mainchan *mc = container_of(chan, mainchan, chan); PacketProtocolLayer *ppl = mc->ppl; /* for ppl_logevent */ - if (!mc->eof_sent & (seat_eof(mc->ppl->seat) || mc->got_pty)) { + if (!mc->eof_sent && (seat_eof(mc->ppl->seat) || mc->got_pty)) { /* * Either seat_eof told us that the front end wants us to * close the outgoing side of the connection as soon as we see diff --git a/ssh2transport.c b/ssh2transport.c index 7b04e5f7..152d530d 100644 --- a/ssh2transport.c +++ b/ssh2transport.c @@ -1903,11 +1903,11 @@ static void ssh2_transport_reconfigure(PacketProtocolLayer *ppl, Conf *conf) if (s->max_data_size < old_max_data_size) { unsigned long diff = old_max_data_size - s->max_data_size; - /* Intentionally use bitwise OR instead of logical, so - * that we decrement both counters even if the first one - * runs out */ - if ((DTS_CONSUME(s->stats, out, diff) != 0) | - (DTS_CONSUME(s->stats, in, diff) != 0)) + /* We must decrement both counters, so avoid short-circuit + * evaluation skipping one */ + int out_expired = DTS_CONSUME(s->stats, out, diff) != 0; + int in_expired = DTS_CONSUME(s->stats, in, diff) != 0; + if (out_expired || in_expired) rekey_reason = "data limit lowered"; } else { unsigned long diff = s->max_data_size - old_max_data_size; diff --git a/unix/gtkask.c b/unix/gtkask.c index 97b78941..5b45d342 100644 --- a/unix/gtkask.c +++ b/unix/gtkask.c @@ -407,7 +407,7 @@ static const char *gtk_askpass_setup(struct askpass_ctx *ctx, ctx->cols[2].red = ctx->cols[2].green = ctx->cols[2].blue = 0x8000; gdk_colormap_alloc_colors(ctx->colmap, ctx->cols, 2, FALSE, TRUE, success); - if (!success[0] | !success[1]) + if (!success[0] || !success[1]) return "unable to allocate colours"; } #endif