Re: [PATCH kmod 05/13] libkmod: nuke struct file_ops

[Lucas De Marchi <lucas.demarchi@intel.com>]

On Mon, Feb 12, 2024 at 05:23:06PM GMT, Emil Velikov via B4 Relay wrote:
>From: Emil Velikov <emil.l.velikov@gmail.com>
>
>With the previous commits, we removed the need for a distinct unload
>callback.
>
>So nuke the struct all together and only use/keep the load one around.
>
>Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
>---


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

thanks
Lucas De Marchi

> libkmod/libkmod-file.c | 62 +++++++++++++++-----------------------------------
> 1 file changed, 18 insertions(+), 44 deletions(-)
>
>diff --git a/libkmod/libkmod-file.c b/libkmod/libkmod-file.c
>index b408aed..8a0336f 100644
>--- a/libkmod/libkmod-file.c
>+++ b/libkmod/libkmod-file.c
>@@ -41,18 +41,12 @@
> #include "libkmod.h"
> #include "libkmod-internal.h"
>
>-struct kmod_file;
>-struct file_ops {
>-	int (*load)(struct kmod_file *file);
>-	void (*unload)(struct kmod_file *file);
>-};
>-
> struct kmod_file {
> 	int fd;
> 	enum kmod_file_compression_type compression;
> 	off_t size;
> 	void *memory;
>-	const struct file_ops *ops;
>+	int (*load)(struct kmod_file *file);
> 	const struct kmod_ctx *ctx;
> 	struct kmod_elf *elf;
> };
>@@ -181,11 +175,6 @@ out:
> 	return ret;
> }
>
>-static void unload_zstd(struct kmod_file *file)
>-{
>-	free(file->memory);
>-}
>-
> static const char magic_zstd[] = {0x28, 0xB5, 0x2F, 0xFD};
> #endif
>
>@@ -287,11 +276,6 @@ static int load_xz(struct kmod_file *file)
> 	return ret;
> }
>
>-static void unload_xz(struct kmod_file *file)
>-{
>-	free(file->memory);
>-}
>-
> static const char magic_xz[] = {0xfd, '7', 'z', 'X', 'Z', 0};
> #endif
>
>@@ -356,11 +340,6 @@ error:
> 	return err;
> }
>
>-static void unload_zlib(struct kmod_file *file)
>-{
>-	free(file->memory);
>-}
>-
> static const char magic_zlib[] = {0x1f, 0x8b};
> #endif
>
>@@ -368,18 +347,18 @@ static const struct comp_type {
> 	size_t magic_size;
> 	enum kmod_file_compression_type compression;
> 	const char *magic_bytes;
>-	const struct file_ops ops;
>+	int (*load)(struct kmod_file *file);
> } comp_types[] = {
> #ifdef ENABLE_ZSTD
>-	{sizeof(magic_zstd),	KMOD_FILE_COMPRESSION_ZSTD, magic_zstd, {load_zstd, unload_zstd}},
>+	{sizeof(magic_zstd),	KMOD_FILE_COMPRESSION_ZSTD, magic_zstd, load_zstd},
> #endif
> #ifdef ENABLE_XZ
>-	{sizeof(magic_xz),	KMOD_FILE_COMPRESSION_XZ, magic_xz, {load_xz, unload_xz}},
>+	{sizeof(magic_xz),	KMOD_FILE_COMPRESSION_XZ, magic_xz, load_xz},
> #endif
> #ifdef ENABLE_ZLIB
>-	{sizeof(magic_zlib),	KMOD_FILE_COMPRESSION_ZLIB, magic_zlib, {load_zlib, unload_zlib}},
>+	{sizeof(magic_zlib),	KMOD_FILE_COMPRESSION_ZLIB, magic_zlib, load_zlib},
> #endif
>-	{0,			KMOD_FILE_COMPRESSION_NONE, NULL, {NULL, NULL}}
>+	{0,			KMOD_FILE_COMPRESSION_NONE, NULL, NULL}
> };
>
> static int load_reg(struct kmod_file *file)
>@@ -400,15 +379,6 @@ static int load_reg(struct kmod_file *file)
> 	return 0;
> }
>
>-static void unload_reg(struct kmod_file *file)
>-{
>-	munmap(file->memory, file->size);
>-}
>-
>-static const struct file_ops reg_ops = {
>-	load_reg, unload_reg
>-};
>-
> struct kmod_elf *kmod_file_get_elf(struct kmod_file *file)
> {
> 	if (file->elf)
>@@ -436,7 +406,7 @@ struct kmod_file *kmod_file_open(const struct kmod_ctx *ctx,
> 		goto error;
> 	}
>
>-	for (itr = comp_types; itr->ops.load != NULL; itr++) {
>+	for (itr = comp_types; itr->load != NULL; itr++) {
> 		if (magic_size_max < itr->magic_size)
> 			magic_size_max = itr->magic_size;
> 	}
>@@ -459,17 +429,17 @@ struct kmod_file *kmod_file_open(const struct kmod_ctx *ctx,
> 			goto error;
> 		}
>
>-		for (itr = comp_types; itr->ops.load != NULL; itr++) {
>+		for (itr = comp_types; itr->load != NULL; itr++) {
> 			if (memcmp(buf, itr->magic_bytes, itr->magic_size) == 0) {
>-				file->ops = &itr->ops;
>+				file->load = itr->load;
> 				file->compression = itr->compression;
> 				break;
> 			}
> 		}
> 	}
>
>-	if (file->ops == NULL) {
>-		file->ops = &reg_ops;
>+	if (file->load == NULL) {
>+		file->load = load_reg;
> 		file->compression = KMOD_FILE_COMPRESSION_NONE;
> 	}
>
>@@ -496,7 +466,7 @@ void kmod_file_load_contents(struct kmod_file *file)
> 		return;
>
> 	/*  The load functions already log possible errors. */
>-	file->ops->load(file);
>+	file->load(file);
> }
>
> void *kmod_file_get_contents(const struct kmod_file *file)
>@@ -524,8 +494,12 @@ void kmod_file_unref(struct kmod_file *file)
> 	if (file->elf)
> 		kmod_elf_unref(file->elf);
>
>-	if (file->memory)
>-		file->ops->unload(file);
>+	if (file->compression == KMOD_FILE_COMPRESSION_NONE) {
>+		if (file->memory)
>+			munmap(file->memory, file->size);
>+	} else {
>+		free(file->memory);
>+	}
>
> 	close(file->fd);
> 	free(file);
>
>-- 
>2.43.0
>