chore: resolve TODOs from Phani

runtime/include/arch/aarch64/context.h

@@ -5,7 +5,11 @@
 #include <ucontext.h>
 
 typedef uint64_t reg_t;
-#define ARCH_NREGS 31 // TODO: aarch64 context-switch assembly and registers!
+#define ARCH_NREGS 31
+
+/**
+ * ARM64 code. Currently Unimplemented
+ **/
 
 /*
  * This is the slowpath switch to a preempted sandbox!

runtime/include/arch/x86_64/context.h

@@ -122,8 +122,6 @@ arch_context_switch(arch_context_t *ca, arch_context_t *na)
 	reg_t *cr = ca->regs, *nr = na->regs;
 	assert(cr && nr);
 
-	/* TODO: slowpath: signal the current pthread if resuming a preempted sandbox! */
-
 	asm volatile("pushq %%rbp\n\t"
 	             "movq %%rsp, %%rbp\n\t"
 	             "movq $2f, 128(%%rax)\n\t"

runtime/include/http.h

@@ -17,8 +17,7 @@ struct http_request {
 	int                header_count;
 	char *             body;
 	int                body_length;
-	// TODO: What does bodyrlen mean? Does this suggest that I've misunderstood what body_length is?
+	int 			   body_read_length; // How far we've read
-	int 			   bodyrlen;
 	// additional for http-parser
 	int last_was_value; // http-parser flag used to help the http-parser callbacks differentiate between header fields and values to know when to allocate a new header
 	int header_end;     // boolean flag set when header processing is complete

runtime/include/module.h

@@ -160,7 +160,6 @@ module__main(struct module *module, i32 argc, i32 argv)
 static inline void
 module__acquire(struct module *module)
 {
-	// TODO: atomic.
 	module->reference_count++;
 }
 
@@ -171,7 +170,6 @@ module__acquire(struct module *module)
 static inline void
 module__release(struct module *module)
 {
-	// TODO: atomic.
 	module->reference_count--;
 }
 

runtime/include/sandbox.h

@@ -70,7 +70,7 @@ struct sandbox {
 	char *  read_buffer;
 	ssize_t read_length, read_size;
 
-	// TODO: Is this used?
+	// Used by the ps_list macro
 	struct ps_list list;
 
 	ssize_t request_response_data_length;      // <= max(module->max_request_or_response_size)
@@ -113,7 +113,6 @@ set_current_sandbox(struct sandbox *sandbox)
 	// Thread Local State about the Current Sandbox
 	sandbox_lmbase  = sandbox->linear_memory_start;
 	sandbox_lmbound = sandbox->linear_memory_size;
-	// TODO: module table or sandbox table?
 	module_indirect_table = sandbox->module->indirect_table;
 }
 

runtime/include/sandbox_request.h

@@ -97,7 +97,6 @@ steal_sandbox_request_from_global_dequeue(void)
 #if NCORES == 1
 	pop_sandbox_request_from_global_dequeue(&sandbox_request);
 #else
-	// TODO: check! is there a sandboxing thread on same core as udp-server thread?
 	int r = deque_steal_sandbox(global_deque, &sandbox_request);
 	if (r) sandbox_request = NULL;
 #endif

runtime/include/types.h

@@ -69,7 +69,6 @@ void populate_memory(void);
 void populate_table(void);
 
 // memory/* also provides the table access functions
-// TODO: Change this to use a compiled in size
 #define INDIRECT_TABLE_SIZE (1 << 10)
 
 struct indirect_table_entry {

runtime/src/env.c

@@ -176,7 +176,6 @@ env___unmapself(u32 base, u32 size)
 
 
 // Floating point routines
-// TODO: Do a fair comparison between musl and wasm-musl
 INLINE double
 env_sin(double d)
 {

runtime/src/http.c

@@ -310,7 +310,8 @@ http_init(void)
 int
 http_request_parse_sb(struct sandbox *sandbox, size_t length)
 {
-	// TODO: Why is our start address sandbox->request_response_data + sandbox->request_response_data_length?
+	// Why is our start address sandbox->request_response_data + sandbox->request_response_data_length?
+	// it's like a cursor to keep track of what we've read so far
 	http_parser_execute(&sandbox->http_parser, &settings, sandbox->request_response_data + sandbox->request_response_data_length, length);
 	return 0;
 }

runtime/src/libc/uvio.c

@@ -109,8 +109,8 @@ wasm_read(i32 filedes, i32 buf_offset, i32 nbyte)
 		struct http_request *r      = &s->http_request;
 		if (r->body_length <= 0) return 0;
 		int l = nbyte > r->body_length ? r->body_length : nbyte;
-		memcpy(buffer, r->body + r->bodyrlen, l);
+		memcpy(buffer, r->body + r->body_read_length, l);
-		r->bodyrlen += l;
+		r->body_read_length += l;
 		r->body_length -= l;
 		return l;
 	}
@@ -506,11 +506,11 @@ wasm_readv(i32 file_descriptor, i32 iov_offset, i32 iovcnt)
 			if (l <= 0) break;
 			char *b = get_memory_ptr_void(iov[i].base_offset, iov[i].len);
 			// http request body!
-			memcpy(b, r->body + r->bodyrlen + len, l);
+			memcpy(b, r->body + r->body_read_length + len, l);
 			len += l;
 			r->body_length -= l;
 		}
-		r->bodyrlen += len;
+		r->body_read_length += len;
 
 		return len;
 	}

runtime/src/module.c

@@ -12,7 +12,7 @@
 
 // In-memory representation of all active modules
 static struct module *__mod_db[MOD_MAX] = { NULL };
-// TODO: What is this?
+// First free in module
 static int            __mod_free_off    = 0;
 
 /**
@@ -57,7 +57,7 @@ add_module(struct module *module)
 {
 	assert(module->socket_descriptor == -1);
 
-	// TODO: Where does this function code from?
+	// __sync_fetch_and_add is provided by GCC
 	int f = __sync_fetch_and_add(&__mod_free_off, 1);
 	assert(f < MOD_MAX);
 	__mod_db[f] = module;

runtime/src/runtime.c

@@ -141,7 +141,6 @@ __thread static struct ps_list_head local_completion_queue;
 __thread sandbox_t *current_sandbox = NULL;
 
 // context pointer to switch to when this thread gets a SIGUSR1
-// TODO: Delete this? It doesn't seem to be used.
 __thread arch_context_t *next_context = NULL;
 
 // context of the runtime thread before running sandboxes or to resume its "main".
@@ -188,7 +187,6 @@ block_current_sandbox(void)
 	assert(in_callback == 0);
 	softint_disable();
 	struct sandbox *current_sandbox = get_current_sandbox();
-	// TODO: What is this getting removed from again? the thread-local runqueue?
 	ps_list_rem_d(current_sandbox);
 	current_sandbox->state          = BLOCKED;
 	struct sandbox *next_sandbox = get_next_sandbox_from_local_run_queue(0);
@@ -289,7 +287,6 @@ add_sandbox_to_local_run_queue(struct sandbox *sandbox)
 
 /**
  * Removes the thread from the thread-local runqueue
- * TODO: is this correct?
  * @param sandbox sandbox
  **/
 static inline void
@@ -419,7 +416,6 @@ worker_thread_main(void *return_code)
  * Called when the function in the sandbox exits
  * Removes the standbox from the thread-local runqueue, sets its state to RETURNED,
  * releases the linear memory, and then switches to the sandbox at the head of the runqueue
- * TODO: Why are we not adding to the completion queue here? That logic is commented out.
  * TODO: Does this belong in sandbox.c?
  **/
 void
@@ -437,6 +433,5 @@ exit_current_sandbox(void)
 	// free resources from "main function execution", as stack still in use.
 	// unmap linear memory only!
 	munmap(current_sandbox->linear_memory_start, SBOX_MAX_MEM + PAGE_SIZE);
-	// add_sandbox_to_completion_queue(current_sandbox);
 	switch_to_sandbox(next_sandbox);
 }

runtime/src/sandbox.c

@@ -166,8 +166,7 @@ setup_sandbox_arguments(i32 argument_count)
 
 /**
  * Receive and Parse the Request for the current sandbox
- * @return 1 on success, < 0 on failure. 
+ * @return 1 on success, 0 if no context, < 0 on failure. 
- * TODO: What does 0 mean? 
  **/
 static inline int
 receive_and_parse_current_sandbox_client_request(void)
@@ -324,12 +323,10 @@ sandbox_main(void)
 	if (receive_and_parse_current_sandbox_client_request() > 0) {
 
 		//
-		current_sandbox->request_response_data_length = response_header_length; // TODO: do this on first write to body.
+		current_sandbox->request_response_data_length = response_header_length;
 
 		// Allocate the WebAssembly Sandbox
 		alloc_linear_memory();
-		// perhaps only initialized for the first instance? or TODO!
-		// module__initialize_table(current_module);
 		module__initialize_globals(current_module);
 		module__initialize_memory(current_module);
 
@@ -395,7 +392,6 @@ free_sandbox(struct sandbox *sandbox)
 	if (!sandbox || sandbox == get_current_sandbox()) return;
 
 	// again sandbox should be done and waiting for the parent.
-	// TODO: this needs to be enhanced. you may be killing a sandbox when its in any other execution states.
 	if (sandbox->state != RETURNED) return;
 
 	int sz = sizeof(struct sandbox);
@@ -403,7 +399,6 @@ free_sandbox(struct sandbox *sandbox)
 	sz += sandbox->module->max_request_or_response_size;
 	module__release(sandbox->module);
 
-	// TODO free(sandbox->arguments);
 	void * stkaddr = sandbox->stack_start;
 	size_t stksz   = sandbox->stack_size;
 
@@ -416,7 +411,6 @@ free_sandbox(struct sandbox *sandbox)
 
 	// remove stack!
 	// for some reason, removing stack seem to cause crash in some cases.
-	// TODO: debug more.
 	ret = munmap(stkaddr, stksz);
 	if (ret) perror("munmap stack");
 }

runtime/src/softint.c

@@ -102,7 +102,7 @@ extern pthread_t worker_threads[];
 /**
  * The handler function for Software Interrupts (signals)
  * SIGALRM is executed periodically by an interval timer, causing preemption of the current sandbox
- * SIGUSR1 does TODO: ???TODO: ???
+ * SIGUSR1 restores a preempted sandbox
  * @param signal_type
  * @param signal_info data structure containing signal info
  * @param user_context_raw void* to a user_context struct

runtime/tools/udpclient/udpclient.c

@@ -59,7 +59,6 @@ send_fn(void *d)
 		return NULL;
 	}
 
-	// todo: select rcv from!
 	int sa_len = sizeof(sa);
 	if (recvfrom(file_descriptor, resp, STR_MAX, 0, (struct sockaddr *)&sa, &sa_len) < 0) { perror("recvfrom"); }
 	printf("Done[%s]!\n", resp);