libkmod: Improve index dump performance

Use FILE for output to reduce the amount of system calls. Removes the
only use case of write_str_safe, which can be removed as well.

Signed-off-by: Tobias Stoeckmann <[email protected]>

libkmod/libkmod-index.c

@@ -360,7 +360,7 @@ static struct index_node_f *index_readchild(const struct index_node_f *parent, i
 	return NULL;
 }
 
-static void index_dump_node(struct index_node_f *node, struct strbuf *buf, int fd)
+static void index_dump_node(struct index_node_f *node, struct strbuf *buf, FILE *fp)
 {
 	struct index_value *v;
 	size_t pushed;
@@ -369,10 +369,10 @@ static void index_dump_node(struct index_node_f *node, struct strbuf *buf, int f
 	pushed = strbuf_pushchars(buf, node->prefix);
 
 	for (v = node->values; v != NULL; v = v->next) {
-		write_str_safe(fd, buf->bytes, buf->used);
-		write_str_safe(fd, " ", 1);
-		write_str_safe(fd, v->value, strlen(v->value));
-		write_str_safe(fd, "\n", 1);
+		fwrite(buf->bytes, 1, buf->used, fp);
+		fputc(' ', fp);
+		fputs(v->value, fp);
+		fputc('\n', fp);
 	}
 
 	for (ch = node->first; ch <= node->last; ch++) {
@@ -382,7 +382,7 @@ static void index_dump_node(struct index_node_f *node, struct strbuf *buf, int f
 			continue;
 
 		if (strbuf_pushchar(buf, ch)) {
-			index_dump_node(child, buf, fd);
+			index_dump_node(child, buf, fp);
 			strbuf_popchar(buf);
 		}
 	}
@@ -391,7 +391,7 @@ static void index_dump_node(struct index_node_f *node, struct strbuf *buf, int f
 	index_close(node);
 }
 
-void index_dump(struct index_file *in, int fd, bool alias_prefix)
+void index_dump(struct index_file *in, FILE *fp, bool alias_prefix)
 {
 	struct index_node_f *root;
 	struct strbuf buf;
@@ -402,7 +402,7 @@ void index_dump(struct index_file *in, int fd, bool alias_prefix)
 
 	strbuf_init(&buf);
 	if (!alias_prefix || strbuf_pushchars(&buf, "alias "))
-		index_dump_node(root, &buf, fd);
+		index_dump_node(root, &buf, fp);
 	strbuf_release(&buf);
 }
 
@@ -821,7 +821,7 @@ static struct index_mm_node *index_mm_readchild(const struct index_mm_node *pare
 	return NULL;
 }
 
-static void index_mm_dump_node(struct index_mm_node *node, struct strbuf *buf, int fd)
+static void index_mm_dump_node(struct index_mm_node *node, struct strbuf *buf, FILE *fp)
 {
 	const void *p;
 	size_t i, pushed;
@@ -833,10 +833,10 @@ static void index_mm_dump_node(struct index_mm_node *node, struct strbuf *buf, i
 		struct index_mm_value v;
 
 		read_value_mm(&p, &v);
-		write_str_safe(fd, buf->bytes, buf->used);
-		write_str_safe(fd, " ", 1);
-		write_str_safe(fd, v.value, v.len);
-		write_str_safe(fd, "\n", 1);
+		fwrite(buf->bytes, 1, buf->used, fp);
+		fputc(' ', fp);
+		fwrite(v.value, 1, v.len, fp);
+		fputc('\n', fp);
 	}
 
 	for (ch = node->first; ch <= node->last; ch++) {
@@ -847,15 +847,15 @@ static void index_mm_dump_node(struct index_mm_node *node, struct strbuf *buf, i
 			continue;
 
 		if (strbuf_pushchar(buf, ch)) {
-			index_mm_dump_node(child, buf, fd);
+			index_mm_dump_node(child, buf, fp);
 			strbuf_popchar(buf);
 		}
 	}
 
 	strbuf_popchars(buf, pushed);
 }
 
-void index_mm_dump(const struct index_mm *idx, int fd, bool alias_prefix)
+void index_mm_dump(const struct index_mm *idx, FILE *fp, bool alias_prefix)
 {
 	struct index_mm_node nbuf, *root;
 	struct strbuf buf;
@@ -866,7 +866,7 @@ void index_mm_dump(const struct index_mm *idx, int fd, bool alias_prefix)
 
 	strbuf_init(&buf);
 	if (!alias_prefix || strbuf_pushchars(&buf, "alias "))
-		index_mm_dump_node(root, &buf, fd);
+		index_mm_dump_node(root, &buf, fp);
 	strbuf_release(&buf);
 }
 

libkmod/libkmod-index.h

@@ -19,7 +19,7 @@ struct index_file;
 struct index_file *index_file_open(const char *filename);
 void index_file_close(struct index_file *idx);
 char *index_search(struct index_file *idx, const char *key);
-void index_dump(struct index_file *in, int fd, bool alias_prefix);
+void index_dump(struct index_file *in, FILE *fp, bool alias_prefix);
 struct index_value *index_searchwild(struct index_file *idx, const char *key);
 
 void index_values_free(struct index_value *values);
@@ -31,4 +31,4 @@ int index_mm_open(const struct kmod_ctx *ctx, const char *filename,
 void index_mm_close(struct index_mm *index);
 char *index_mm_search(const struct index_mm *idx, const char *key);
 struct index_value *index_mm_searchwild(const struct index_mm *idx, const char *key);
-void index_mm_dump(const struct index_mm *idx, int fd, bool alias_prefix);
+void index_mm_dump(const struct index_mm *idx, FILE *fp, bool alias_prefix);

libkmod/libkmod.c

@@ -810,22 +810,37 @@ KMOD_EXPORT void kmod_unload_resources(struct kmod_ctx *ctx)
 
 KMOD_EXPORT int kmod_dump_index(struct kmod_ctx *ctx, enum kmod_index type, int fd)
 {
+	int err = 0, fd2;
+	FILE *fp;
+
 	if (ctx == NULL)
 		return -ENOSYS;
 
+	fd2 = dup(fd);
+	if (fd2 == -1)
+		return -errno;
+	fp = fdopen(fd2, "ae");
+	if (fp == NULL) {
+		err = -errno;
+		close(fd2);
+		return err;
+	}
+
 #if defined(__clang__)
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wtautological-unsigned-enum-zero-compare"
 #endif
-	if (type < 0 || type >= _KMOD_INDEX_MODULES_SIZE)
-		return -ENOENT;
+	if (type < 0 || type >= _KMOD_INDEX_MODULES_SIZE) {
+		err = -ENOENT;
+		goto done;
+	}
 #if defined(__clang__)
 #pragma clang diagnostic pop
 #endif
 
 	if (ctx->indexes[type] != NULL) {
 		DBG(ctx, "use mmapped index '%s'\n", index_files[type].fn);
-		index_mm_dump(ctx->indexes[type], fd, index_files[type].alias_prefix);
+		index_mm_dump(ctx->indexes[type], fp, index_files[type].alias_prefix);
 	} else {
 		char fn[PATH_MAX];
 		struct index_file *idx;
@@ -835,14 +850,18 @@ KMOD_EXPORT int kmod_dump_index(struct kmod_ctx *ctx, enum kmod_index type, int
 		DBG(ctx, "file=%s\n", fn);
 
 		idx = index_file_open(fn);
-		if (idx == NULL)
-			return -ENOSYS;
+		if (idx == NULL) {
+			err = -ENOSYS;
+			goto done;
+		}
 
-		index_dump(idx, fd, index_files[type].alias_prefix);
+		index_dump(idx, fp, index_files[type].alias_prefix);
 		index_file_close(idx);
 	}
 
-	return 0;
+done:
+	fclose(fp);
+	return err;
 }
 
 const struct kmod_config *kmod_get_config(const struct kmod_ctx *ctx)

shared/util.c

@@ -237,32 +237,6 @@ ssize_t read_str_safe(int fd, char *buf, size_t buflen)
 	return done;
 }
 
-ssize_t write_str_safe(int fd, const char *buf, size_t buflen)
-{
-	size_t todo = buflen;
-	size_t done = 0;
-
-	assert_cc(EAGAIN == EWOULDBLOCK);
-
-	do {
-		ssize_t r = write(fd, buf + done, todo);
-
-		if (r == 0)
-			break;
-		else if (r > 0) {
-			todo -= r;
-			done += r;
-		} else {
-			if (errno == EAGAIN || errno == EINTR)
-				continue;
-			else
-				return -errno;
-		}
-	} while (todo > 0);
-
-	return done;
-}
-
 int read_str_long(int fd, long *value, int base)
 {
 	char buf[32], *end;

shared/util.h

@@ -37,7 +37,6 @@ _nonnull_all_ bool path_ends_with_kmod_ext(const char *path, size_t len);
 _must_check_ _nonnull_(2) ssize_t pread_str_safe(int fd, char *buf, size_t buflen,
 						 off_t off);
 _must_check_ _nonnull_(2) ssize_t read_str_safe(int fd, char *buf, size_t buflen);
-_nonnull_(2) ssize_t write_str_safe(int fd, const char *buf, size_t buflen);
 _must_check_ _nonnull_(2) int read_str_long(int fd, long *value, int base);
 _must_check_ _nonnull_(2) int read_str_ulong(int fd, unsigned long *value, int base);
 _nonnull_(1) char *freadline_wrapped(FILE *fp, unsigned int *linenum);

testsuite/test-util.c

@@ -162,35 +162,6 @@ static int test_path_ends_with_kmod_ext(const struct test *t)
 DEFINE_TEST(test_path_ends_with_kmod_ext,
 	    .description = "check implementation of path_ends_with_kmod_ext()")
 
-#define TEST_WRITE_STR_SAFE_FILE "/write-str-safe"
-#define TEST_WRITE_STR_SAFE_PATH TESTSUITE_ROOTFS "test-util2/" TEST_WRITE_STR_SAFE_FILE
-static int test_write_str_safe(const struct test *t)
-{
-	const char *s = "test";
-	int fd;
-
-	fd = open(TEST_WRITE_STR_SAFE_FILE ".txt", O_CREAT | O_TRUNC | O_WRONLY, 0644);
-	assert_return(fd >= 0, EXIT_FAILURE);
-
-	write_str_safe(fd, s, strlen(s));
-	close(fd);
-
-	return EXIT_SUCCESS;
-}
-DEFINE_TEST(test_write_str_safe,
-	.description = "check implementation of write_str_safe()",
-	.config = {
-		[TC_ROOTFS] = TESTSUITE_ROOTFS "test-util2/",
-	},
-	.need_spawn = true,
-	.output = {
-		.files = (const struct keyval[]) {
-			{ TEST_WRITE_STR_SAFE_PATH ".txt",
-			  TEST_WRITE_STR_SAFE_PATH "-correct.txt" },
-			{ },
-		},
-	});
-
 static int test_uadd32_overflow(const struct test *t)
 {
 	uint32_t res;