- 

From 490cb06ef878bff1e45acf313105205df7baced1 Mon Sep 17 00:00:00 2001

From: Eygene Ryabinkin <rea-fbsd@codelabs.ru>

Date: Sun, 21 Dec 2008 15:18:11 +0300

Subject: [PATCH] Add processing of both READ and WRITE events inside libevent handlers

If both read and write descriptors are ready, then libevent will set

both EV_READ and EV_WRITE.  Original OpenOSPFD sources were not ready to

handle such situations, but OpenBSD's libevent (1.3e with some local

tweaks) and even libevent 1.0 can also produce combined read/write

events (if manual page is correct ;).

Such errors were seen in the wild, for example Remko Lodder had been

biten by this issue.

I had created inline function that will be called from both 6 places

that are handling READ/WRITE events to eliminate code duplication.

Signed-off-by: Eygene Ryabinkin <rea-fbsd@codelabs.ru>

---

 ospfd/libevent_helpers.h |   81 ++++++++++++++++++++++++++++++++++++++++++++++

 ospfd/ospfd.c            |   31 ++----------------

 ospfd/ospfe.c            |   39 ++++-----------------

 ospfd/rde.c              |   31 ++----------------

 4 files changed, 95 insertions(+), 87 deletions(-)

 create mode 100644 ospfd/libevent_helpers.h

diff --git a/ospfd/libevent_helpers.h b/ospfd/libevent_helpers.h

new file mode 100644

index 0000000..46f743d

--- /dev/null

+++ ospfd/libevent_helpers.h

@@ -0,0 +1,81 @@

+/*

+ * Copyright (c) 2008 Eygene Ryabinkin <rea-fbsd@codelabs.ru>

+ *

+ * Permission to use, copy, modify, and distribute this software for any

+ * purpose with or without fee is hereby granted, provided that the above

+ * copyright notice and this permission notice appear in all copies.

+ *

+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES

+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF

+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR

+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES

+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN

+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF

+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

+ */

+

+#ifndef _LIBEVENT_HELPERS_H_

+#define _LIBEVENT_HELPERS_H_

+

+#include <stdio.h>

+#include <sys/types.h>

+

+#include "log.h"

+#include "ospfd.h"

+

+/* Inline functions */

+

+/*

+ * A support function that processes libevent notification in the

+ * following way:

+ * - if we are ready to write, we will try to flush the queue;

+ * - if we are ready to read, we will read the input buffer and

+ *   prepare variables 'n' and 'shut' accordingly.

+ *

+ * Such handling occurs at least 6 times within the OSPFD sources,

+ * so this inline function is just an alternative to the preprocessor

+ * macros.

+ *

+ * Function returns 0 if everything was handled and no further

+ * processing is needed; it returns EV_READ if the read processing

+ * was prepared to take place.

+ */

+static inline short

+dispatch_read_write_event(short _event, struct imsgbuf *_ibuf,

+    ssize_t *_n, int *_shut) __attribute__((always_inline));

+

+static inline short

+dispatch_read_write_event(short event, struct imsgbuf *ibuf,

+    ssize_t *n, int *shut)

+{

+	static char errbuf[128];

+

+	/*

+	 * We can have both EV_READ and EV_WRITE, since we can be

+	 * subscribed to both event types.  Handle write readiness

+	 * first (flush the queue) and then handle reads.

+	 */

+	if ((event & ~(EV_WRITE|EV_READ))) {

+		snprintf(errbuf, sizeof(errbuf),

+		    "unknown event 0x%hx", (unsigned short)event);

+		fatalx(errbuf);

+	}

+

+	if ((event & EV_WRITE)) {

+		if (msgbuf_write(&ibuf->w) == -1)

+			fatal("msgbuf_write");

+		imsg_event_add(ibuf);

+	}

+	if ((event & EV_READ)) {

+		if ((*n = imsg_read(ibuf)) == -1)

+			fatal("imsg_read error");

+		if (*n == 0)	/* connection closed */

+			*shut = 1;

+		return EV_READ;

+	} else {

+		return 0;

+	}

+	/* NOTREACHED */

+}

+

+#endif	/* _LIBEVENT_HELPERS_H_ */

diff --git a/ospfd/ospfd.c b/ospfd/ospfd.c

index be69cab..4e62d09 100644

--- ospfd/ospfd.c

+++ ospfd/ospfd.c

@@ -46,6 +46,7 @@

 #include "control.h"

 #include "log.h"

 #include "rde.h"

+#include "libevent_helpers.h"

 void		main_sig_handler(int, short, void *);

 __dead void	usage(void);

@@ -355,21 +356,8 @@ main_dispatch_ospfe(int fd, short event, void *bula)

 	ssize_t			 n;

 	int			 shut = 0;

-	switch (event) {

-	case EV_READ:

-		if ((n = imsg_read(ibuf)) == -1)

-			fatal("imsg_read error");

-		if (n == 0)	/* connection closed */

-			shut = 1;

-		break;

-	case EV_WRITE:

-		if (msgbuf_write(&ibuf->w) == -1)

-			fatal("msgbuf_write");

-		imsg_event_add(ibuf);

+	if (dispatch_read_write_event(event, ibuf, &n, &shut) != EV_READ)

 		return;

-	default:

-		fatalx("unknown event");

-	}

 	for (;;) {

 		if ((n = imsg_get(ibuf, &imsg)) == -1)

@@ -434,21 +422,8 @@ main_dispatch_rde(int fd, short event, void *bula)

 	ssize_t		 n;

 	int		 count, shut = 0;

-	switch (event) {

-	case EV_READ:

-		if ((n = imsg_read(ibuf)) == -1)

-			fatal("imsg_read error");

-		if (n == 0)	/* connection closed */

-			shut = 1;

-		break;

-	case EV_WRITE:

-		if (msgbuf_write(&ibuf->w) == -1)

-			fatal("msgbuf_write");

-		imsg_event_add(ibuf);

+	if (dispatch_read_write_event(event, ibuf, &n, &shut) != EV_READ)

 		return;

-	default:

-		fatalx("unknown event");

-	}

 	for (;;) {

 		if ((n = imsg_get(ibuf, &imsg)) == -1)

diff --git a/ospfd/ospfe.c b/ospfd/ospfe.c

index af7a406..d6a6aa9 100644

--- ospfd/ospfe.c

+++ ospfd/ospfe.c

@@ -42,6 +42,7 @@

 #include "rde.h"

 #include "control.h"

 #include "log.h"

+#include "libevent_helpers.h"

 void		 ospfe_sig_handler(int, short, void *);

 void		 ospfe_shutdown(void);

@@ -257,23 +258,11 @@ ospfe_dispatch_main(int fd, short event, void *bula)

 	struct iface	*iface = NULL;

 	struct kif	*kif;

 	struct auth_md	 md;

-	int		 n, link_ok, stub_changed, shut = 0;

-

-	switch (event) {

-	case EV_READ:

-		if ((n = imsg_read(ibuf)) == -1)

-			fatal("imsg_read error");

-		if (n == 0)	/* connection closed */

-			shut = 1;

-		break;

-	case EV_WRITE:

-		if (msgbuf_write(&ibuf->w) == -1)

-			fatal("msgbuf_write");

-		imsg_event_add(ibuf);

+	int		 link_ok, stub_changed, shut = 0;

+	ssize_t		 n;

+

+	if (dispatch_read_write_event(event, ibuf, &n, &shut) != EV_READ)

 		return;

-	default:

-		fatalx("unknown event");

-	}

 	for (;;) {

 		if ((n = imsg_get(ibuf, &imsg)) == -1)

@@ -401,24 +390,12 @@ ospfe_dispatch_rde(int fd, short event, void *bula)

 	struct lsa_entry	*le;

 	struct imsg		 imsg;

 	struct abr_rtr		 ar;

-	int			 n, noack = 0, shut = 0;

+	int			 noack = 0, shut = 0;

 	u_int16_t		 l, age;

+	ssize_t			 n;

-	switch (event) {

-	case EV_READ:

-		if ((n = imsg_read(ibuf)) == -1)

-			fatal("imsg_read error");

-		if (n == 0)	/* connection closed */

-			shut = 1;

-		break;

-	case EV_WRITE:

-		if (msgbuf_write(&ibuf->w) == -1)

-			fatal("msgbuf_write");

-		imsg_event_add(ibuf);

+	if (dispatch_read_write_event(event, ibuf, &n, &shut) != EV_READ)

 		return;

-	default:

-		fatalx("unknown event");

-	}

 	for (;;) {

 		if ((n = imsg_get(ibuf, &imsg)) == -1)

diff --git a/ospfd/rde.c b/ospfd/rde.c

index 5dd0623..faa0c23 100644

--- ospfd/rde.c

+++ ospfd/rde.c

@@ -37,6 +37,7 @@

 #include "ospfe.h"

 #include "log.h"

 #include "rde.h"

+#include "libevent_helpers.h"

 void		 rde_sig_handler(int sig, short, void *);

 void		 rde_shutdown(void);

@@ -239,21 +240,8 @@ rde_dispatch_imsg(int fd, short event, void *bula)

 	int			 r, state, self, shut = 0;

 	u_int16_t		 l;

-	switch (event) {

-	case EV_READ:

-		if ((n = imsg_read(ibuf)) == -1)

-			fatal("imsg_read error");

-		if (n == 0)	/* connection closed */

-			shut = 1;

-		break;

-	case EV_WRITE:

-		if (msgbuf_write(&ibuf->w) == -1)

-			fatal("msgbuf_write");

-		imsg_event_add(ibuf);

+	if (dispatch_read_write_event(event, ibuf, &n, &shut) != EV_READ)

 		return;

-	default:

-		fatalx("unknown event");

-	}

 	clock_gettime(CLOCK_MONOTONIC, &tp);

 	now = tp.tv_sec;

@@ -584,21 +572,8 @@ rde_dispatch_parent(int fd, short event, void *bula)

 	ssize_t			 n;

 	int			 shut = 0;

-	switch (event) {

-	case EV_READ:

-		if ((n = imsg_read(ibuf)) == -1)

-			fatal("imsg_read error");

-		if (n == 0)	/* connection closed */

-			shut = 1;

-		break;

-	case EV_WRITE:

-		if (msgbuf_write(&ibuf->w) == -1)

-			fatal("msgbuf_write");

-		imsg_event_add(ibuf);

+	if (dispatch_read_write_event(event, ibuf, &n, &shut) != EV_READ)

 		return;

-	default:

-		fatalx("unknown event");

-	}

 	for (;;) {

 		if ((n = imsg_get(ibuf, &imsg)) == -1)

-- 

1.6.0.5