From 285188963d06b5838148b961ab7b6bb5c6e3b0e5 Mon Sep 17 00:00:00 2001
From: David Paschich <davidp@navdy.com>
Date: Sat, 21 May 2016 21:05:11 -0700
Subject: [PATCH] Fix bufferevent_pair to properly set
 BEV_EVENT_{READING,WRITING} on flush.

Here's some fun. From `bufferevent.h`:
  ```
  #define BEV_EVENT_READING	0x01	/**< error encountered while reading */
  #define BEV_EVENT_WRITING	0x02	/**< error encountered while writing */
  ```

And from `event.h`:
  ```
  /** Wait for a socket or FD to become readable */
  #define EV_READ		0x02
  /** Wait for a socket or FD to become writeable */
  #define EV_WRITE	0x04
  ```

Library users have to be very careful to get this right; it turns out, the
library itself got this wrong in the `bufferevent_pair` code. It appears that
in most of the code, only `BEV_EVENT_FINISHED` will indicate whether it's read
or write; on error or timeout, it appears that "both" is assumed and not set in
the callback. I read through all the other places where `BEV_EVENT_FINISHED` is
passed to an event callback; it appears that the pair code is the only spot
that got it wrong.

azat: add TT_FORK to avoid breaking clean env, and rebase commit message
(copied from #359)
Fixes: #359
---
 bufferevent_pair.c         |  7 ++++++-
 test/regress_bufferevent.c | 39 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/bufferevent_pair.c b/bufferevent_pair.c
index d80e5f81..1e93f572 100644
--- a/bufferevent_pair.c
+++ b/bufferevent_pair.c
@@ -325,7 +325,12 @@ be_pair_flush(struct bufferevent *bev, short iotype,
 		be_pair_transfer(bev, partner, 1);
 
 	if (mode == BEV_FINISHED) {
-		bufferevent_run_eventcb_(partner, iotype|BEV_EVENT_EOF, 0);
+		short what = BEV_EVENT_EOF;
+		if (iotype & EV_READ)
+			what |= BEV_EVENT_WRITING;
+		if (iotype & EV_WRITE)
+			what |= BEV_EVENT_READING;
+		bufferevent_run_eventcb_(partner, what, 0);
 	}
 	decref_and_unlock(bev);
 	return 0;
diff --git a/test/regress_bufferevent.c b/test/regress_bufferevent.c
index 33ddbda3..6f214a7d 100644
--- a/test/regress_bufferevent.c
+++ b/test/regress_bufferevent.c
@@ -1190,6 +1190,42 @@ end:
 		bufferevent_free(bev);
 }
 
+static void
+pair_flush_eventcb(struct bufferevent *bev, short what, void *ctx)
+{
+	int *callback_what = ctx;
+	*callback_what = what;
+}
+
+static void
+test_bufferevent_pair_flush(void *arg)
+{
+	struct basic_test_data *data = arg;
+	struct bufferevent *pair[2];
+	struct bufferevent *bev1 = NULL;
+	struct bufferevent *bev2 = NULL;
+	int callback_what = 0;
+
+	tt_assert(0 == bufferevent_pair_new(data->base, 0, pair));
+	bev1 = pair[0];
+	bev2 = pair[1];
+	tt_assert(0 == bufferevent_enable(bev1, EV_WRITE));
+	tt_assert(0 == bufferevent_enable(bev2, EV_READ));
+
+	bufferevent_setcb(bev2, NULL, NULL, pair_flush_eventcb, &callback_what);
+
+	bufferevent_flush(bev1, EV_WRITE, BEV_FINISHED);
+
+	event_base_loop(data->base, EVLOOP_ONCE);
+
+	tt_assert(callback_what == (BEV_EVENT_READING | BEV_EVENT_EOF));
+
+end:
+	if (bev1)
+		bufferevent_free(bev1);
+	if (bev2)
+		bufferevent_free(bev2);
+}
 
 struct testcase_t bufferevent_testcases[] = {
 
@@ -1260,6 +1296,9 @@ struct testcase_t bufferevent_testcases[] = {
 	{ "bufferevent_socket_filter_inactive",
 	  test_bufferevent_socket_filter_inactive,
 	  TT_FORK|TT_NEED_BASE, &basic_setup, NULL },
+	{ "bufferevent_pair_flush",
+	  test_bufferevent_pair_flush,
+	  TT_FORK|TT_NEED_BASE, &basic_setup, NULL },
 
 	END_OF_TESTCASES,
 };
-- 
2.40.0