GopherProxy

	Refactor queue-event-handling to be pointer-centric - quark - quark web server
	git clone git://git.suckless.org/quark
	Log
	Files
	Refs
	LICENSE
	---
	commit 35e3b69d60f337724163e9543b5728b907ce34dd
	parent f1892c4dffa7df6995cd8c9ebd47e0ce7f0e457e
	Author: Laslo Hunhold <[email protected]>
	Date: Sun, 31 Jan 2021 11:33:46 +0100

	Refactor queue-event-handling to be pointer-centric

	This is one aspect where we can see that kqueue(2) is superior to
	epoll(7). While kqueue(2) allows you to both store a pointer and read
	out the fd of an event in the ready-list, epoll(7) only provides you
	with one of the two (by only offering a union-type-field where you
	either store an fd or a pointer).

	Previously, wrapper functions would allow to extract either from any
	event, which was only non-hacky in the OpenBSD-case, because nothing
	stopped you from reading the "fd" from the union that actually was
	assigned as a pointer. The distinction was there in the first place
	because we are listening on the main incoming socket and other "client"-
	sockets at the same time. The main-socket-events contained the socket-fd
	and the client-sockets contained the connection-struct-pointer. Now, we
	just set the data-pointer of the main socket to NULL to indicate it,
	yielding a much more refined usage with one fewer way to screw things
	up.

	Signed-off-by: Laslo Hunhold <[email protected]>

	Diffstat:
	M main.c \| 31 +++++++++++++++--------------…
	M queue.c \| 32 +++++------------------------…
	M queue.h \| 3 +--

	3 files changed, 21 insertions(+), 45 deletions(-)
	---
	diff --git a/main.c b/main.c
	@@ -210,9 +210,9 @@ static void *
	thread_method(void *data)
	{
	queue_event *event = NULL;
	- struct connection connection, c;
	+ struct connection connection, c, *newc;
	struct worker_data d = (struct worker_data )data;
	- int qfd, fd;
	+ int qfd;
	ssize_t nready;
	size_t i;

	@@ -226,7 +226,7 @@ thread_method(void *data)
	exit(1);
	}

	- /* add insock to the interest list */
	+ /* add insock to the interest list (with data=NULL) */
	if (queue_add_fd(qfd, d->insock, QUEUE_EVENT_IN, 1, NULL) < 0) {
	exit(1);
	}
	@@ -244,12 +244,12 @@ thread_method(void *data)

	/* handle events */
	for (i = 0; i < (size_t)nready; i++) {
	- if (queue_event_is_dropped(&event[i])) {
	- fd = queue_event_get_fd(&event[i]);
	+ c = queue_event_get_data(&event[i]);

	- if (fd != d->insock) {
	- memset(queue_event_get_ptr(&event[i]),
	- 0, sizeof(struct connection));
	+ if (queue_event_is_dropped(&event[i])) {
	+ if (c != NULL) {
	+ queue_rem_fd(qfd, c->fd);
	+ close_connection(c);
	}

	printf("dropped a connection\n");
	@@ -257,11 +257,11 @@ thread_method(void *data)
	continue;
	}

	- if (queue_event_get_fd(&event[i]) == d->insock) {
	+ if (c == NULL) {
	/* add new connection to the interest list */
	- if (!(c = accept_connection(d->insock,
	- connection,
	- d->nslots))) {
	+ if (!(newc = accept_connection(d->insock,
	+ connection,
	+ d->nslots))) {
	/*
	* the socket is either blocking
	* or something failed.
	@@ -275,20 +275,19 @@ thread_method(void *data)
	* (we want IN, because we start
	* with receiving the header)
	*/
	- if (queue_add_fd(qfd, c->fd,
	+ if (queue_add_fd(qfd, newc->fd,
	QUEUE_EVENT_IN,
	- 0, c) < 0) {
	+ 0, newc) < 0) {
	/* not much we can do here */
	continue;
	}
	} else {
	- c = queue_event_get_ptr(&event[i]);
	-
	/* serve existing connection */
	serve_connection(c, d->srv);

	if (c->fd == 0) {
	/* we are done */
	+ memset(c, 0, sizeof(struct connection)…
	continue;
	}

	diff --git a/queue.c b/queue.c
	@@ -65,14 +65,8 @@ queue_add_fd(int qfd, int fd, enum queue_event_type t, int s…
	break;
	}

	- /* set data */
	- if (data == NULL) {
	- /* the data is the fd itself */
	- e.data.fd = fd;
	- } else {
	- /* set data pointer */
	- e.data.ptr = (void *)data;
	- }
	+ /* set data pointer */
	+ e.data.ptr = (void *)data;

	/* register fd in the interest list */
	if (epoll_ctl(qfd, EPOLL_CTL_ADD, fd, &e) < 0) {
	@@ -124,14 +118,8 @@ queue_mod_fd(int qfd, int fd, enum queue_event_type t, con…
	break;
	}

	- /* set data */
	- if (data == NULL) {
	- /* the data is the fd itself */
	- e.data.fd = fd;
	- } else {
	- /* set data pointer */
	- e.data.ptr = (void *)data;
	- }
	+ /* set data pointer */
	+ e.data.ptr = (void *)data;

	/* register fd in the interest list */
	if (epoll_ctl(qfd, EPOLL_CTL_MOD, fd, &e) < 0) {
	@@ -208,18 +196,8 @@ queue_wait(int qfd, queue_event *e, size_t elen)
	return nready;
	}

	-int
	-queue_event_get_fd(const queue_event *e)
	-{
	- #ifdef __linux__
	- return e->data.fd;
	- #else
	- return e->ident;
	- #endif
	-}
	-
	void *
	-queue_event_get_ptr(const queue_event *e)
	+queue_event_get_data(const queue_event *e)
	{
	#ifdef __linux__
	return e->data.ptr;
	diff --git a/queue.h b/queue.h
	@@ -26,8 +26,7 @@ int queue_mod_fd(int, int, enum queue_event_type, const void …
	int queue_rem_fd(int, int);
	ssize_t queue_wait(int, queue_event *, size_t);

	-int queue_event_get_fd(const queue_event *);
	-void queue_event_get_ptr(const queue_event );
	+void queue_event_get_data(const queue_event );

	int queue_event_is_dropped(const queue_event *e);