From ce00d1677d7a419b427e7f11963eee982a55a231 Mon Sep 17 00:00:00 2001
From: dzwdz
Date: Thu, 4 Aug 2022 23:06:57 +0200
Subject: do some simple TODOs, organize the rest; general code maintainance

---
 src/kernel/arch/amd64/driver/fsroot.c       | 12 +----------
 src/kernel/arch/amd64/driver/util.c         | 16 +++++++++++++++
 src/kernel/arch/amd64/driver/util.h         |  4 ++++
 src/kernel/arch/amd64/driver/video.c        | 15 ++------------
 src/kernel/arch/amd64/interrupts/isr_stub.s |  2 +-
 src/kernel/arch/amd64/paging.h              |  2 +-
 src/kernel/arch/amd64/sysenter.s            |  2 +-
 src/kernel/handle.c                         |  2 --
 src/kernel/mem/alloc.c                      |  3 ++-
 src/kernel/proc.c                           |  2 --
 src/kernel/syscalls.c                       | 14 ++++++-------
 src/kernel/vfs/mount.c                      | 31 ++++++++++++++---------------
 src/kernel/vfs/path.h                       |  3 +--
 src/kernel/vfs/request.c                    |  3 ++-
 14 files changed, 53 insertions(+), 58 deletions(-)
 create mode 100644 src/kernel/arch/amd64/driver/util.c
 create mode 100644 src/kernel/arch/amd64/driver/util.h

(limited to 'src/kernel')

diff --git a/src/kernel/arch/amd64/driver/fsroot.c b/src/kernel/arch/amd64/driver/fsroot.c
index 512b7da..41fe136 100644
--- a/src/kernel/arch/amd64/driver/fsroot.c
+++ b/src/kernel/arch/amd64/driver/fsroot.c
@@ -2,6 +2,7 @@
 #include <camellia/fsutil.h>
 #include <kernel/arch/amd64/ata.h>
 #include <kernel/arch/amd64/driver/fsroot.h>
+#include <kernel/arch/amd64/driver/util.h>
 #include <kernel/mem/virt.h>
 #include <kernel/panic.h>
 #include <kernel/proc.h>
@@ -22,17 +23,6 @@ static bool exacteq(struct vfs_request *req, const char *str) {
 	return req->input.len == len && !memcmp(req->input.buf_kern, str, len);
 }
 
-static int req_readcopy(struct vfs_request *req, const void *buf, size_t len) {
-	assert(req->type == VFSOP_READ);
-	fs_normslice(&req->offset, &req->output.len, len, false);
-	virt_cpy_to(
-			req->caller->pages, req->output.buf,
-			buf + req->offset, req->output.len);
-	/* read errors are ignored. TODO write docs */
-	return req->output.len;
-}
-
-
 static int handle(struct vfs_request *req) {
 	assert(req->caller);
 	int id = (int)(long __force)req->id;
diff --git a/src/kernel/arch/amd64/driver/util.c b/src/kernel/arch/amd64/driver/util.c
new file mode 100644
index 0000000..b03e582
--- /dev/null
+++ b/src/kernel/arch/amd64/driver/util.c
@@ -0,0 +1,16 @@
+#include <camellia/fsutil.h>
+#include <kernel/arch/amd64/driver/util.h>
+#include <kernel/mem/virt.h>
+#include <kernel/panic.h>
+#include <kernel/vfs/request.h>
+
+int req_readcopy(struct vfs_request *req, const void *buf, size_t len) {
+	if (!req->caller) return -1;
+	assert(req->type == VFSOP_READ);
+	fs_normslice(&req->offset, &req->output.len, len, false);
+	virt_cpy_to(
+			req->caller->pages, req->output.buf,
+			buf + req->offset, req->output.len);
+	/* read errors are ignored. TODO write a spec */
+	return req->output.len;
+}
diff --git a/src/kernel/arch/amd64/driver/util.h b/src/kernel/arch/amd64/driver/util.h
new file mode 100644
index 0000000..fd8d506
--- /dev/null
+++ b/src/kernel/arch/amd64/driver/util.h
@@ -0,0 +1,4 @@
+#pragma once
+
+struct vfs_request;
+int req_readcopy(struct vfs_request *req, const void *buf, size_t len);
diff --git a/src/kernel/arch/amd64/driver/video.c b/src/kernel/arch/amd64/driver/video.c
index 8ace3d4..2d1ac03 100644
--- a/src/kernel/arch/amd64/driver/video.c
+++ b/src/kernel/arch/amd64/driver/video.c
@@ -1,5 +1,6 @@
-#include <camellia/fsutil.h>
 #include <camellia/errno.h>
+#include <camellia/fsutil.h>
+#include <kernel/arch/amd64/driver/util.h>
 #include <kernel/arch/amd64/driver/video.h>
 #include <kernel/mem/virt.h>
 #include <kernel/panic.h>
@@ -12,18 +13,6 @@ enum {
 	H_FB,
 };
 
-/* stolen from fsroot.c, TODO shared copy? i guess? */
-static int req_readcopy(struct vfs_request *req, const void *buf, size_t len) {
-	if (!req->caller) return -1;
-	assert(req->type == VFSOP_READ);
-	fs_normslice(&req->offset, &req->output.len, len, false);
-	virt_cpy_to(
-			req->caller->pages, req->output.buf,
-			buf + req->offset, req->output.len);
-	/* read errors are ignored. TODO write docs */
-	return req->output.len;
-}
-
 static int handle(struct vfs_request *req) {
 	switch (req->type) {
 		case VFSOP_OPEN:
diff --git a/src/kernel/arch/amd64/interrupts/isr_stub.s b/src/kernel/arch/amd64/interrupts/isr_stub.s
index e45c1c1..c63bf2c 100644
--- a/src/kernel/arch/amd64/interrupts/isr_stub.s
+++ b/src/kernel/arch/amd64/interrupts/isr_stub.s
@@ -75,7 +75,7 @@ _isr_stage2:
 	iretq
 
 .align 8
-// TODO overflow check
+// TODO stack overflow check
 .skip 256 // seems to be enough
 .global _isr_mini_stack
 _isr_mini_stack:
diff --git a/src/kernel/arch/amd64/paging.h b/src/kernel/arch/amd64/paging.h
index f8de339..156c0c5 100644
--- a/src/kernel/arch/amd64/paging.h
+++ b/src/kernel/arch/amd64/paging.h
@@ -31,7 +31,7 @@ typedef union pe_generic_t {
 	void *as_ptr;
 } pe_generic_t; // pageentry_generic
 
-struct pagedir { // on amd64 actually points to pml4. TODO more sensible type
+struct pagedir { /* on amd64 actually points to pml4. the name is like this for historical reasons */
 	pe_generic_t e[512];
 } __attribute__((packed));
 
diff --git a/src/kernel/arch/amd64/sysenter.s b/src/kernel/arch/amd64/sysenter.s
index 1dfb422..8fa8acc 100644
--- a/src/kernel/arch/amd64/sysenter.s
+++ b/src/kernel/arch/amd64/sysenter.s
@@ -36,7 +36,7 @@ stored_rsp:
 	.skip 8
 
 .global pagedir_current
-// a hack to maintain compat with the old arch api, TODO
+// TODO make into an argument of sysexit
 pagedir_current:
 	.skip 8
 
diff --git a/src/kernel/handle.c b/src/kernel/handle.c
index ed7896d..3e31a12 100644
--- a/src/kernel/handle.c
+++ b/src/kernel/handle.c
@@ -39,8 +39,6 @@ void handle_close(struct handle *h) {
 
 	// TODO sanity check to check if refcount is true. handle_sanity?
 
-	// TODO tests which would catch premature frees
-	// by that i mean duplicating a handle and killing the original process
 	h->type = HANDLE_INVALID;
 	kfree(h);
 }
diff --git a/src/kernel/mem/alloc.c b/src/kernel/mem/alloc.c
index e3eaf41..a6f6b6d 100644
--- a/src/kernel/mem/alloc.c
+++ b/src/kernel/mem/alloc.c
@@ -102,7 +102,8 @@ void *page_zalloc(size_t pages) {
 
 // frees `pages` consecutive pages starting from *first
 void page_free(void *first_addr, size_t pages) {
-	if (first_addr < page_bitmap_start) // TODO unimplemented
+	// TODO put init's pages into the allocator
+	if (first_addr < page_bitmap_start)
 		return;
 	size_t first = (uintptr_t)(first_addr - page_bitmap_start) / PAGE_SIZE;
 	for (size_t i = 0; i < pages; i++) {
diff --git a/src/kernel/proc.c b/src/kernel/proc.c
index 3bf2b0d..6bc7057 100644
--- a/src/kernel/proc.c
+++ b/src/kernel/proc.c
@@ -65,7 +65,6 @@ struct process *process_fork(struct process *parent, int flags) {
 	parent->handled_req = NULL;
 
 	if ((flags & FORK_NEWFS) == 0 && parent->controlled) {
-		// TODO would it be better to change the default to not sharing the controlled fs?
 		child->controlled = parent->controlled;
 		child->controlled->potential_handlers++;
 		child->controlled->refcount++;
@@ -149,7 +148,6 @@ void process_kill(struct process *p, int ret) {
 			p->execbuf.buf = NULL;
 		}
 
-		if (p->parent)
 			pagedir_free(p->pages); // TODO put init's pages in the allocator
 
 		// TODO VULN unbounded recursion
diff --git a/src/kernel/syscalls.c b/src/kernel/syscalls.c
index ceba528..0ab6106 100644
--- a/src/kernel/syscalls.c
+++ b/src/kernel/syscalls.c
@@ -1,4 +1,5 @@
 #include <camellia/errno.h>
+#include <camellia/execbuf.h>
 #include <camellia/flags.h>
 #include <camellia/syscalls.h>
 #include <kernel/arch/generic.h>
@@ -32,7 +33,7 @@ long _syscall_await(void) {
 	{
 		if (iter->noreap) continue;
 		has_children = true;
-		if (iter->state == PS_DEAD) // TODO this path used to crash, still untested
+		if (iter->state == PS_DEAD)
 			SYSCALL_RETURN(process_try2collect(iter));
 	}
 
@@ -276,10 +277,9 @@ long _syscall_fs_wait(char __user *buf, long max_len, struct fs_wait_response __
 	if (!backend) SYSCALL_RETURN(-1);
 
 	process_transition(process_current, PS_WAITS4REQUEST);
-	assert(!backend->user.handler); // TODO allow multiple processes to wait on the same backend
+	if (backend->user.handler)
+		panic_unimplemented();
 	backend->user.handler = process_current;
-	/* checking the validity of those pointers here would make
-	 * vfs_backend_accept simpler. TODO? */
 	process_current->awaited_req.buf     = buf;
 	process_current->awaited_req.max_len = max_len;
 	process_current->awaited_req.res     = res;
@@ -370,11 +370,11 @@ void _syscall_sleep(long ms) {
 
 long _syscall_execbuf(void __user *ubuf, size_t len) {
 	if (len == 0) SYSCALL_RETURN(0);
-	if (len > sizeof(uint64_t) * 6 * 4) // TODO specify max size somewhere
+	if (len > EXECBUF_MAX_LEN)
 		SYSCALL_RETURN(-1);
 	if (process_current->execbuf.buf)
-		SYSCALL_RETURN(-1); /* no nesting */
-	// actually TODO, nesting makes sense for infinite loops. maybe
+		SYSCALL_RETURN(-1);
+	// TODO consider supporting nesting execbufs
 
 	char *kbuf = kmalloc(len);
 	if (!virt_cpy_from(process_current->pages, kbuf, ubuf, len)) {
diff --git a/src/kernel/vfs/mount.c b/src/kernel/vfs/mount.c
index 77cc14b..de2d708 100644
--- a/src/kernel/vfs/mount.c
+++ b/src/kernel/vfs/mount.c
@@ -3,13 +3,25 @@
 #include <kernel/vfs/mount.h>
 #include <shared/mem.h>
 
-// TODO not the place where this should be done
 static struct vfs_mount *mount_root = NULL;
 
 struct vfs_mount *vfs_mount_seed(void) {
 	return mount_root;
 }
 
+void vfs_mount_root_register(const char *path, struct vfs_backend *backend) {
+	struct vfs_mount *mount = kmalloc(sizeof *mount);
+	*mount = (struct vfs_mount){
+		.prev = mount_root,
+		.prefix = path,
+		.prefix_len = strlen(path),
+		.backend = backend,
+		.refs = 1,
+	};
+	mount_root = mount;
+}
+
+
 struct vfs_mount *vfs_mount_resolve(
 		struct vfs_mount *top, const char *path, size_t path_len)
 {
@@ -20,9 +32,8 @@ struct vfs_mount *vfs_mount_resolve(
 			continue;
 
 		/* ensure that there's no garbage after the match
-		 * recognize that e.g. /thisasdf doesn't get recognized as mounted under
-		 * /this */
-		
+		 * e.g. don't recognize /thisasdf as mounted under /this */
+
 		if (top->prefix_len == path_len) // can't happen if prefix == path
 			break;
 		if (path[top->prefix_len] == '/')
@@ -45,15 +56,3 @@ void vfs_mount_remref(struct vfs_mount *mnt) {
 
 	if (prev) vfs_mount_remref(prev);
 }
-
-void vfs_mount_root_register(const char *path, struct vfs_backend *backend) {
-	struct vfs_mount *mount = kmalloc(sizeof *mount);
-	*mount = (struct vfs_mount){
-		.prev = mount_root,
-		.prefix = path,
-		.prefix_len = strlen(path),
-		.backend = backend,
-		.refs = 1,
-	};
-	mount_root = mount;
-}
diff --git a/src/kernel/vfs/path.h b/src/kernel/vfs/path.h
index 8efa0d4..7484619 100644
--- a/src/kernel/vfs/path.h
+++ b/src/kernel/vfs/path.h
@@ -1,8 +1,7 @@
 #pragma once
+#include <camellia/path.h>
 #include <stddef.h>
 
-#define PATH_MAX 512
-
 /** Reduce a path to its simplest form.
  *
  * @return length of the string in *out, always less than len. Negative if the path was invalid.
diff --git a/src/kernel/vfs/request.c b/src/kernel/vfs/request.c
index d942fba..6883faa 100644
--- a/src/kernel/vfs/request.c
+++ b/src/kernel/vfs/request.c
@@ -114,7 +114,8 @@ void vfs_backend_user_accept(struct vfs_request *req) {
 	assert(req && req->backend && req->backend->user.handler);
 	handler = req->backend->user.handler;
 	assert(handler->state == PS_WAITS4REQUEST);
-	assert(handler->handled_req == NULL);
+	if (handler->handled_req)
+		panic_unimplemented();
 
 	// the virt_cpy calls aren't present in all kernel backends
 	// it's a way to tell apart kernel and user backends apart
-- 
cgit v1.2.3