kernel: start cleaning up VfsRequest

* I'm being more strict about the linked list state to hopefully ensure
  I'm not leaking any references.
* vfsreq_create was renamed to vfsreq_dispatchcopy as that name feels more
  clear. It copies its argument, and dispatches it.
* Requests for user backends are now handled more like requests for kernel
  backends - there's an accept() function that accepts a request.

2 files changed, 86 insertions, 47 deletions

diff --git a/src/kernel/vfs/request.c b/src/kernel/vfs/request.c
index 8fe0114..4489f5a 100644
--- a/src/kernel/vfs/request.c
+++ b/src/kernel/vfs/request.c
@@ -6,45 +6,75 @@
 #include <kernel/vfs/request.h>
 #include <shared/mem.h>
 
-static void vfs_backend_user_accept(VfsReq *req);
+/** Checks if the request flags don't contradict each other.
+ * While this could be done by caller of vfsreq_dispatchcopy, as currently
+ * there's only one of them per operation, doing this here means that any
+ * potential future callers can't forget those checks.
+ * Doing this so late is kinda inefficient but I don't really care. */
+static bool
+vfsreq_isvalid(VfsReq *r)
+{
+	/* Notable omissions:
+	 * - Simplifying the path for open(). Already done when resolving the mount.
+	 * - Checking the handle permissions. We don't have access to it.
+	 * */
+	if (r->type == VFSOP_OPEN && !(r->flags & OPEN_WRITE) && (r->flags & OPEN_CREATE)) {
+		return false;
+	}
+
+	// XXX if i add a handle field to vfs_request, check ->readable ->writeable here
+	return true;
+}
 
-void vfsreq_create(VfsReq req_) {
+void
+vfsreq_dispatchcopy(VfsReq tmpl)
+{
 	VfsReq *req;
-	if (req_.caller) {
-		proc_setstate(req_.caller, PS_WAITS4FS);
-		if (!req_.caller->reqslot)
-			req_.caller->reqslot = kmalloc(sizeof *req, "reqslot");
-		req = req_.caller->reqslot;
+	VfsBackend *backend;
+
+	/* allocate memory for the request and move it there */
+	if (tmpl.caller) {
+		// TODO: ensure we aren't overriding an existing request
+		// TODO: wouldn't it be better to just optimize malloc?
+		//       see commit afa28cfc7069f7649632f7b33daac47ee56d3819
+		proc_setstate(tmpl.caller, PS_WAITS4FS);
+		if (!tmpl.caller->reqslot)
+			tmpl.caller->reqslot = kmalloc(sizeof *req, "reqslot");
+		req = tmpl.caller->reqslot;
 		/* (re)using a single allocation for all request a process makes */
 	} else {
 		req = kmalloc(sizeof *req, "reqanon");
 	}
-	memcpy(req, &req_, sizeof *req);
-	if (req->backend) {
-		assert(req->backend->usehcnt);
-		req->backend->usehcnt++;
+
+	memcpy(req, &tmpl, sizeof *req);
+	backend = req->backend;
+	if (backend) {
+		assert(backend->usehcnt > 0);
+		backend->usehcnt++;
 	}
 
-	if (req->type == VFSOP_OPEN && !(req->flags & OPEN_WRITE) && (req->flags & OPEN_CREATE)) {
+	/* Check request validity. Doing this after the memcpy means that it
+	 * doesn't need to be special cased in vfsreq_finish. */
+	if (!vfsreq_isvalid(req)) {
 		vfsreq_finish_short(req, -EINVAL);
 		return;
 	}
-
-	// TODO if i add a handle field to vfs_request, check ->readable ->writeable here
-
-	if (req->backend && req->backend->provhcnt) {
-		VfsReq **iter = &req->backend->queue;
-		while (*iter != NULL) // find free spot in queue
-			iter = &(*iter)->queue_next;
-		*iter = req;
-		vfs_backend_tryaccept(req->backend);
-	} else {
-		vfsreq_finish_short(req, -1);
+	assert(req->queue_next == NULL);
+	assert(req->postqueue_next == NULL);
+
+	if (backend == NULL) {
+		/* null mount - probably never had a real backend */
+		vfsreq_finish_short(req, -ENOENT);
+	} else if (backend->is_user) {
+		vfsback_useraccept(req);
+	} else { /* kernel backend */
+		assert(backend->kern.accept);
+		backend->kern.accept(req);
 	}
 }
 
-void vfsreq_finish(VfsReq *req, char __user *stored, long ret,
-		int flags, Proc *handler)
+void
+vfsreq_finish(VfsReq *req, char __user *stored, long ret, int flags, Proc *handler)
 {
 	if (req->type == VFSOP_OPEN && ret >= 0) {
 		Handle *h;
@@ -88,6 +118,9 @@ void vfsreq_finish(VfsReq *req, char __user *stored, long ret,
 		vfsback_userdown(req->backend);
 	}
 
+	assert(req->queue_next == NULL);
+	assert(req->postqueue_next == NULL);
+
 	if (req->caller) {
 		assert(req->caller->state == PS_WAITS4FS);
 		regs_savereturn(&req->caller->regs, ret);
@@ -97,27 +130,33 @@ void vfsreq_finish(VfsReq *req, char __user *stored, long ret,
 	}
 }
 
-void vfs_backend_tryaccept(VfsBackend *backend) {
-	VfsReq *req = backend->queue;
-	if (!req) return;
-	if (backend->is_user && !backend->user.handler) return;
-
-	backend->queue = req->queue_next;
-	if (backend->is_user) {
-		vfs_backend_user_accept(req);
-	} else {
-		assert(backend->kern.accept);
-		backend->kern.accept(req);
-	}
-}
-
-static void vfs_backend_user_accept(VfsReq *req) {
+void
+vfsback_useraccept(VfsReq *req)
+{
+	VfsBackend *backend;
 	Proc *handler;
 	struct ufs_request res = {0};
 	int len;
 
-	assert(req && req->backend && req->backend->user.handler);
-	handler = req->backend->user.handler;
+	assert(req != NULL);
+	backend = req->backend;
+	assert(backend);
+	assert(backend->is_user);
+	if (backend->provhcnt == 0) {
+		vfsreq_finish_short(req, -EPIPE);
+		return;
+	} else if (backend->user.handler == NULL) {
+		/* queue the request up */
+		// TODO use postqueue
+		VfsReq **it = &backend->queue;
+		while (*it != NULL) { /* find a free spot in queue */
+			it = &(*it)->queue_next;
+		}
+		*it = req;
+		return;
+	}
+
+	handler = backend->user.handler;
 	assert(handler->state == PS_WAITS4REQUEST);
 
 	// the pcpy calls aren't present in all kernel backends
@@ -158,7 +197,6 @@ static void vfs_backend_user_accept(VfsReq *req) {
 	proc_setstate(handler, PS_RUNNING);
 	regs_savereturn(&handler->regs, hid);
 	req->backend->user.handler = NULL;
-	return;
 }
 
 static void
@@ -204,7 +242,8 @@ vfsback_provdown(VfsBackend *b)
 		VfsReq *q = b->queue;
 		while (q) {
 			VfsReq *q2 = q->queue_next;
-			vfsreq_finish_short(q, -1);
+			q->queue_next = NULL;
+			vfsreq_finish_short(q, -EPIPE);
 			q = q2;
 		}
 		b->queue = NULL;
diff --git a/src/kernel/vfs/request.h b/src/kernel/vfs/request.h
index 2bd8c61..0b93491 100644
--- a/src/kernel/vfs/request.h
+++ b/src/kernel/vfs/request.h
@@ -3,7 +3,6 @@
 #include <stdbool.h>
 #include <stddef.h>
 
-/* describes something which can act as an access function */
 struct VfsBackend {
 	/* amount of using references
 	 *  VfsMount
@@ -14,6 +13,7 @@ struct VfsBackend {
 	/* amount of providing references
 	 *  Proc
 	 * 0 - orphaned, will never increase */
+	// TODO move this into .user
 	size_t provhcnt;
 
 	VfsReq *queue;
@@ -64,7 +64,7 @@ struct VfsReq {
 };
 
 /** Assigns the vfs_request to the caller, and dispatches the call */
-void vfsreq_create(VfsReq);
+void vfsreq_dispatchcopy(VfsReq);
 void vfsreq_finish(VfsReq*, char __user *stored, long ret, int flags, Proc *handler);
 
 static inline void vfsreq_finish_short(VfsReq *req, long ret) {
@@ -72,7 +72,7 @@ static inline void vfsreq_finish_short(VfsReq *req, long ret) {
 }
 
 /** Try to accept an enqueued request */
-void vfs_backend_tryaccept(VfsBackend *);
+void vfsback_useraccept(VfsReq *);
 
 /** Decrements the "user" reference count. */
 void vfsback_userdown(VfsBackend *);