1 files changed, 172 insertions, 108 deletions

diff --git a/lib9p/srv.c b/lib9p/srv.c
index e938dcb..295bc1d 100644
--- a/lib9p/srv.c
+++ b/lib9p/srv.c
@@ -711,11 +711,13 @@ static inline void _srv_respond(struct srv_req *ctx, enum lib9p_msg_type resp_ty
 
 /* handle_T* ******************************************************************/
 
-#define srv_handler_common(ctx, typ, req)                               \
+#define srv_handler_common_no_err(ctx, typ, req)                        \
 	assert(ctx);                                                    \
 	assert(req);                                                    \
 	struct lib9p_msg_T##typ *_typecheck_req [[maybe_unused]] = req; \
-	struct lib9p_msg_R##typ resp = { .tag = ctx->tag };             \
+	struct lib9p_msg_R##typ resp = { .tag = ctx->tag }
+#define srv_handler_common(ctx, typ, req)         \
+	srv_handler_common_no_err(ctx, typ, req); \
 	error err = {}
 
 static void handle_Tversion(struct srv_req *ctx,
@@ -1175,25 +1177,33 @@ static inline struct lib9p_stat srv_stat_to_net_stat(struct lib9p_srv_stat in) {
 	};
 }
 
+static void handle_read_file(struct srv_req *ctx, struct srv_fidinfo *fidinfo, uint64_t offset, uint32_t count);
+static void handle_read_dir(struct srv_req *ctx, struct srv_fidinfo *fidinfo, uint64_t offset, uint32_t count);
+
 static void handle_Tread(struct srv_req *ctx,
                          struct lib9p_msg_Tread *req) {
-	srv_handler_common(ctx, read, req);
-	char *heap = NULL;
+	assert(ctx);
+	assert(req);
 
 	/* TODO: serialize simultaneous reads to the same FID */
 
+	/* req->count <= CONFIG_9P_SRV_MAX_MSG_SIZE <= CONFIG_9P_SRV_MAX_HOSTMSG_SIZE <= SIZE_MAX */
+	assert(req->count <= SIZE_MAX);
+	/* req->offset is u64, uoff is u64 */
+	static_assert(req->offset <= UOFF_MAX);
+
 	if (req->count > ctx->basectx.max_msg_size - lib9p_version_min_Rread_size(ctx->basectx.version))
 		req->count = ctx->basectx.max_msg_size - lib9p_version_min_Rread_size(ctx->basectx.version);
 
 	/* Check that the FID is valid for this.  */
 	struct srv_fidinfo *fidinfo = map_load(&ctx->parent_sess->fids, req->fid);
 	if (!fidinfo) {
-		err = error_new(E_POSIX_EBADF, "bad file number ", req->fid);
-		goto tread_return;
+		srv_respond(ctx, read, NULL, error_new(E_POSIX_EBADF, "bad file number ", req->fid));
+		return;
 	}
 	if (!(fidinfo->flags & FIDFLAG_OPEN_R)) {
-		err = error_new(E_POSIX_EINVAL, "FID not open for reading");
-		goto tread_return;
+		srv_respond(ctx, read, NULL, error_new(E_POSIX_EINVAL, "FID not open for reading"));
+		return;
 	}
 
 	/* Do it.  */
@@ -1201,119 +1211,174 @@ static void handle_Tread(struct srv_req *ctx,
 	switch (fidinfo->type) {
 	case SRV_FILETYPE_DIR:
 #if _LIB9P_ENABLE_stat
-		/* Seek.  */
-		if (req->offset == 0) {
-			fidinfo->dir.idx = 0;
-			fidinfo->dir.off = 0;
-			fidinfo->dir.buffered_dirent = (struct lib9p_srv_dirent){};
-		} else if (req->offset != fidinfo->dir.off) {
-			err = error_new(E_POSIX_EINVAL, "invalid offset (must be 0 or ", fidinfo->dir.off, "): ", req->offset);
-			goto tread_return;
-		}
-		/* Read.  */
-		resp.data = heap = malloc(req->count);
-		resp.count = 0;
-		struct srv_pathinfo *dir_pathinfo = NULL;
-		for (;;) {
-			lo_interface lib9p_srv_file member_file = {};
-			struct lib9p_srv_dirent member_dirent;
-			if (fidinfo->dir.buffered_dirent.name.len) {
-				member_dirent = fidinfo->dir.buffered_dirent;
-			} else {
-				lib9p_srv_dirent_or_error member_dirent_r;
-				member_dirent_r = LO_CALL(fidinfo->dir.io, dread, ctx, fidinfo->dir.idx);
-				if (member_dirent_r.is_err) {
-					if (!resp.count) {
-						err = member_dirent_r.err;
-						goto tread_return;
-					}
-					error_cleanup(&member_dirent_r.err);
+		handle_read_dir(ctx, fidinfo, req->offset, req->count);
+#else
+		assert_notreached("Tread for directory on protocol version without that");
+#endif
+		break;
+	case SRV_FILETYPE_FILE:
+		handle_read_file(ctx, fidinfo, req->offset, req->count);
+		break;
+	case SRV_FILETYPE_AUTH:
+		assert_notreached("TODO: auth not yet implemented");
+		break;
+	}
+	ctx->user = srv_userid_decref(ctx->user);
+}
+
+struct rread_writer {
+	struct srv_req  *ctx;
+	size_t           count;
+	bool             written;
+
+};
+LO_IMPLEMENTATION_STATIC(io_writer, struct rread_writer, rread);
+
+static size_t_and_error rread_writev(struct rread_writer *self, const struct iovec *iov, int iovcnt) {
+	assert(self);
+	assert(!self->written);
+	assert(iovcnt == 1);
+	assert(iov);
+	assert(iov->iov_len <= self->count);
+
+	struct lib9p_msg_Rread resp = {
+		.tag = self->ctx->tag,
+		.count = iov->iov_len,
+		.data = iov->iov_base,
+	};
+
+	srv_respond(self->ctx, read, &resp, ERROR_NULL);
+
+	self->written = true;
+	return ERROR_AND(size_t, iov->iov_len, ERROR_NULL);
+}
+
+static void handle_read_file(struct srv_req *ctx, struct srv_fidinfo *fidinfo, uint64_t offset, uint32_t count) {
+	struct rread_writer _writer = {
+		.ctx     = ctx,
+		.count   = (size_t) count,
+		.written = false,
+	};
+	lo_interface io_writer writer = LO_BOX(io_writer, &_writer);
+	error err = LO_CALL(fidinfo->file.io, pread, ctx, writer, offset, count);
+	assert(ERROR_IS_NULL(err) == _writer.written);
+	if (!ERROR_IS_NULL(err))
+		srv_respond(ctx, read, NULL, err);
+}
+
+#if _LIB9P_ENABLE_stat
+static void handle_read_dir(struct srv_req *ctx, struct srv_fidinfo *fidinfo, uint64_t offset, uint32_t count) {
+	/* Seek.  */
+	if (offset == 0) {
+		fidinfo->dir.idx = 0;
+		fidinfo->dir.off = 0;
+		fidinfo->dir.buffered_dirent = (struct lib9p_srv_dirent){};
+	} else if (offset != fidinfo->dir.off) {
+		srv_respond(ctx, read, NULL, error_new(E_POSIX_EINVAL, "invalid offset (must be 0 or ", fidinfo->dir.off, "): ", offset));
+		return;
+	}
+
+	/* Allocate.  */
+	[[gnu::cleanup(heap_cleanup)]] void *heap = NULL;
+	struct lib9p_msg_Rread resp = {
+		.tag = ctx->tag,
+		.data = heap = malloc(count),
+		.count = 0,
+	};
+
+	/* Read.  */
+	struct srv_pathinfo *dir_pathinfo = NULL;
+	for (;;) {
+		/* 1. Call ->dread() to get `member_dirent`.  */
+		struct lib9p_srv_dirent member_dirent;
+		if (fidinfo->dir.buffered_dirent.name.len) {
+			member_dirent = fidinfo->dir.buffered_dirent;
+		} else {
+			lib9p_srv_dirent_or_error r = LO_CALL(fidinfo->dir.io, dread, ctx, fidinfo->dir.idx);
+			if (r.is_err) {
+				if (resp.count) {
+					/* Just do a short-read; discard the error.  */
+					error_cleanup(&r.err);
 					break;
 				}
-				member_dirent = member_dirent_r.lib9p_srv_dirent;
+				srv_respond(ctx, read, NULL, r.err);
+				return;
 			}
-			if (!member_dirent.name.len)
-				break;
-			struct lib9p_srv_stat member_stat;
+			member_dirent = r.lib9p_srv_dirent;
+		}
+		if (!member_dirent.name.len)
+			/* end-of-directory */
+			break;
+
+		/* 2. Call ->dwalk() to get the `member_file` object to call ->stat() on.  */
+		lo_interface lib9p_srv_file member_file;
+		bool free_member_file;
+		{
 			struct srv_pathinfo *member_pathinfo = map_load(&ctx->parent_sess->paths, member_dirent.qid.path);
 			if (member_pathinfo) {
-				lib9p_srv_stat_or_error r = LO_CALL(member_pathinfo->file, stat, ctx);
-				if (r.is_err) {
-					err = r.err;
-					goto member_err;
-				}
-				member_stat = r.lib9p_srv_stat;
+				member_file = member_pathinfo->file;
+				free_member_file = false;
 			} else {
 				if (!dir_pathinfo)
 					dir_pathinfo = map_load(&ctx->parent_sess->paths, fidinfo->path);
 				assert(dir_pathinfo);
-				lib9p_srv_file_or_error file_r = LO_CALL(dir_pathinfo->file, dwalk, ctx, member_dirent.name);
-				if (file_r.is_err) {
-					err = file_r.err;
-					goto member_err;
-				}
-				member_file = file_r.lib9p_srv_file;
-				lib9p_srv_stat_or_error stat_r = LO_CALL(member_file, stat, ctx);
-				if (stat_r.is_err) {
-					err = stat_r.err;
-					goto member_err;
+				lib9p_srv_file_or_error r = LO_CALL(dir_pathinfo->file, dwalk, ctx, member_dirent.name);
+				if (r.is_err) {
+					if (resp.count) {
+						/* Just do a short-read; discard the error.  */
+						error_cleanup(&r.err);
+						break;
+					}
+					srv_respond(ctx, read, NULL, r.err);
+					return;
 				}
-				member_stat = stat_r.lib9p_srv_stat;
-			}
-			if (false) {
-			member_err:
-				if (!LO_IS_NULL(member_file))
-					LO_CALL(member_file, free);
-				if (!resp.count)
-					goto tread_return;
-				error_cleanup(&err);
-				break;
+				member_file = r.lib9p_srv_file;
+				free_member_file = true;
 			}
-			lib9p_srv_stat_assert(member_stat);
-			struct lib9p_stat member_netstat = srv_stat_to_net_stat(member_stat);
-			uint32_t nbytes = lib9p_stat_marshal(&ctx->basectx, req->count-resp.count, &member_netstat,
-			                                     (uint8_t *)&resp.data[resp.count]);
-			if (!LO_IS_NULL(member_file))
+		}
+
+		/* 3. Call ->stat() to get `member_stat``.  */
+		struct lib9p_srv_stat member_stat;
+		{
+			lib9p_srv_stat_or_error r = LO_CALL(member_file, stat, ctx);
+			if (free_member_file)
 				LO_CALL(member_file, free);
-			if (!nbytes) {
-				if (!resp.count) {
-					err = error_new(E_POSIX_ERANGE, "stat object does not fit into negotiated max message size");
-					goto tread_return;
+			if (r.is_err) {
+				if (resp.count) {
+					/* Just do a short-read; discard the error.  */
+					error_cleanup(&r.err);
+					break;
 				}
+				srv_respond(ctx, read, NULL, r.err);
+				return;
+			}
+			member_stat = r.lib9p_srv_stat;
+			lib9p_srv_stat_assert(member_stat);
+		}
+
+		/* 4. Encode `member_stat` into `resp.data` and increment `resp.count`.  */
+		struct lib9p_stat member_netstat = srv_stat_to_net_stat(member_stat);
+		uint32_t nbytes = lib9p_stat_marshal(&ctx->basectx, count-resp.count, &member_netstat,
+		                                     (uint8_t *)&resp.data[resp.count]);
+		if (!nbytes) {
+			if (resp.count) {
+				/* Just do a short-read; discard the error.
+				 * But save the member_dirent for next time.  */
 				fidinfo->dir.buffered_dirent = member_dirent;
 				break;
 			}
-			resp.count += nbytes;
-			fidinfo->dir.idx++;
-			fidinfo->dir.off += nbytes;
-			fidinfo->dir.buffered_dirent = (struct lib9p_srv_dirent){};
-		}
-#else
-		assert_notreached("Tread for directory on protocol version without that");
-#endif
-		break;
-	case SRV_FILETYPE_FILE:
-		iovec_or_error iov = LO_CALL(fidinfo->file.io, pread, ctx, req->count, req->offset);
-		if (iov.is_err) {
-			err = iov.err;
-		} else {
-			resp.count = iov.iovec.iov_len;
-			resp.data = iov.iovec.iov_base;
-			if (resp.count > req->count)
-				resp.count = req->count;
+			srv_respond(ctx, read, NULL,
+			            error_new(E_POSIX_ERANGE, "stat object does not fit into negotiated max message size"));
+			return;
 		}
-		break;
-	case SRV_FILETYPE_AUTH:
-		assert_notreached("TODO: auth not yet implemented");
-		break;
+		resp.count += nbytes;
+		fidinfo->dir.idx++;
+		fidinfo->dir.off += nbytes;
+		fidinfo->dir.buffered_dirent = (struct lib9p_srv_dirent){};
 	}
- tread_return:
-	if (ctx->user)
-		ctx->user = srv_userid_decref(ctx->user);
-	srv_respond(ctx, read, &resp, err);
-	if (heap)
-		free(heap);
+	srv_respond(ctx, read, &resp, ERROR_NULL);
 }
+#endif
 
 static void handle_Twrite(struct srv_req *ctx,
                           struct lib9p_msg_Twrite *req) {
@@ -1349,18 +1414,17 @@ static void handle_Twrite(struct srv_req *ctx,
 
 static void handle_Tclunk(struct srv_req *ctx,
                           struct lib9p_msg_Tclunk *req) {
-	srv_handler_common(ctx, clunk, req);
+	srv_handler_common_no_err(ctx, clunk, req);
 
 	struct srv_fidinfo *fidinfo = map_load(&ctx->parent_sess->fids, req->fid);
 	if (!fidinfo) {
-		err = error_new(E_POSIX_EBADF, "bad file number ", req->fid);
-		goto tclunk_return;
+		srv_respond(ctx, clunk, NULL, error_new(E_POSIX_EBADF, "bad file number ", req->fid));
+		return;
 	}
 
+	srv_respond(ctx, clunk, &resp, ERROR_NULL);
+	/* Yes, don't actually perform the clunk until *after* we send Rclunk.  */
 	srv_fid_del(ctx, req->fid, fidinfo, false);
-
- tclunk_return:
-	srv_respond(ctx, clunk, &resp, err);
 }
 
 static void handle_Tremove(struct srv_req *ctx,