[PATCH 0/4] kmod: Paper cuts - dead code removal, don't SED manually

[PATCH 0/4] kmod: Paper cuts - dead code removal, don't SED manually

[Emil Velikov]

Greetings everyone,

Here's another, final, round of paper cuts while browsing through the project.

Note that despite the enabled linker garbage collector, the first few cleanup
patches result in detectable improvement in the final binaries.

The last patch, removes the open-coded replacement and by doing so fixes a
genuine bug - albeit one that is uncommon to hit.

As always, feel free to pick any bits that seem suitable.

Thanks
Emil

Emil Velikov (4):
  libkmod: remove unused kmod_module_get_builtin
  libkmod: annotate kmod_builtin_iter API as static
  shared: annotate local API as static
  configure: manage libkmod.pc.in and version.py.in via AC_CONFIG_FILES

 Makefile.am                | 25 ----------------------
 configure.ac               |  2 ++
 libkmod/libkmod-builtin.c  |  8 +++----
 libkmod/libkmod-internal.h |  6 ------
 libkmod/libkmod-module.c   | 43 --------------------------------------
 shared/util.c              |  6 +++---
 shared/util.h              |  3 ---
 7 files changed, 9 insertions(+), 84 deletions(-)

-- 
2.39.2

[PATCH 1/4] libkmod: remove unused kmod_module_get_builtin

[Emil Velikov]

From: Emil Velikov <emil.velikov@collabora.com>

The last and only user was removed with commit 0246e06 ("depmod: Stop
opening modules.modinfo once per module")

Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
 libkmod/libkmod-internal.h |  1 -
 libkmod/libkmod-module.c   | 43 --------------------------------------
 2 files changed, 44 deletions(-)

diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h
index c22644a..95c0377 100644
--- a/libkmod/libkmod-internal.h
+++ b/libkmod/libkmod-internal.h
@@ -148,7 +148,6 @@ void kmod_module_set_visited(struct kmod_module *mod, bool visited) __attribute_
 void kmod_module_set_builtin(struct kmod_module *mod, bool builtin) __attribute__((nonnull((1))));
 void kmod_module_set_required(struct kmod_module *mod, bool required) __attribute__((nonnull(1)));
 bool kmod_module_is_builtin(struct kmod_module *mod) __attribute__((nonnull(1)));
-int kmod_module_get_builtin(struct kmod_ctx *ctx, struct kmod_list **list) __attribute__((nonnull(1, 2)));
 
 /* libkmod-file.c */
 struct kmod_file *kmod_file_open(const struct kmod_ctx *ctx, const char *filename) _must_check_ __attribute__((nonnull(1,2)));
diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
index c7232e0..1da64b3 100644
--- a/libkmod/libkmod-module.c
+++ b/libkmod/libkmod-module.c
@@ -2943,46 +2943,3 @@ KMOD_EXPORT void kmod_module_dependency_symbols_free_list(struct kmod_list *list
 		list = kmod_list_remove(list);
 	}
 }
-
-/**
- * kmod_module_get_builtin:
- * @ctx: kmod library context
- * @list: where to save the builtin module list
- *
- * Returns: 0 on success or < 0 otherwise.
- */
-int kmod_module_get_builtin(struct kmod_ctx *ctx, struct kmod_list **list)
-{
-	struct kmod_builtin_iter *iter;
-	int err = 0;
-
-	iter = kmod_builtin_iter_new(ctx);
-	if (!iter)
-		return -errno;
-
-	while (kmod_builtin_iter_next(iter)) {
-		struct kmod_module *mod = NULL;
-		char modname[PATH_MAX];
-
-		if (!kmod_builtin_iter_get_modname(iter, modname)) {
-			err = -errno;
-			goto fail;
-		}
-
-		err = kmod_module_new_from_name(ctx, modname, &mod);
-		if (err < 0)
-			goto fail;
-
-		kmod_module_set_builtin(mod, true);
-
-		*list = kmod_list_append(*list, mod);
-	}
-
-	kmod_builtin_iter_free(iter);
-	return err;
-fail:
-	kmod_builtin_iter_free(iter);
-	kmod_module_unref_list(*list);
-	*list = NULL;
-	return err;
-}
-- 
2.39.2

[PATCH 2/4] libkmod: annotate kmod_builtin_iter API as static

[Emil Velikov]

From: Emil Velikov <emil.velikov@collabora.com>

It's no longer used outside the compilation unit, as of last commit.

Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
 libkmod/libkmod-builtin.c  | 8 ++++----
 libkmod/libkmod-internal.h | 5 -----
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/libkmod/libkmod-builtin.c b/libkmod/libkmod-builtin.c
index a002cb5..65334a8 100644
--- a/libkmod/libkmod-builtin.c
+++ b/libkmod/libkmod-builtin.c
@@ -54,7 +54,7 @@ struct kmod_builtin_iter {
 	char *buf;
 };
 
-struct kmod_builtin_iter *kmod_builtin_iter_new(struct kmod_ctx *ctx)
+static struct kmod_builtin_iter *kmod_builtin_iter_new(struct kmod_ctx *ctx)
 {
 	char path[PATH_MAX];
 	int file, sv_errno;
@@ -108,7 +108,7 @@ fail:
 	return iter;
 }
 
-void kmod_builtin_iter_free(struct kmod_builtin_iter *iter)
+static void kmod_builtin_iter_free(struct kmod_builtin_iter *iter)
 {
 	close(iter->file);
 	free(iter->buf);
@@ -165,7 +165,7 @@ fail:
 	return -1;
 }
 
-bool kmod_builtin_iter_next(struct kmod_builtin_iter *iter)
+static bool kmod_builtin_iter_next(struct kmod_builtin_iter *iter)
 {
 	char *line,  *modname;
 	size_t linesz;
@@ -216,7 +216,7 @@ bool kmod_builtin_iter_next(struct kmod_builtin_iter *iter)
 	return (iter->pos < iter->size);
 }
 
-bool kmod_builtin_iter_get_modname(struct kmod_builtin_iter *iter,
+static bool kmod_builtin_iter_get_modname(struct kmod_builtin_iter *iter,
 				char modname[static PATH_MAX])
 {
 	int sv_errno;
diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h
index 95c0377..4a4af58 100644
--- a/libkmod/libkmod-internal.h
+++ b/libkmod/libkmod-internal.h
@@ -198,9 +198,4 @@ bool kmod_module_signature_info(const struct kmod_file *file, struct kmod_signat
 void kmod_module_signature_info_free(struct kmod_signature_info *sig_info) __attribute__((nonnull));
 
 /* libkmod-builtin.c */
-struct kmod_builtin_iter;
-struct kmod_builtin_iter *kmod_builtin_iter_new(struct kmod_ctx *ctx) __attribute__((nonnull(1)));
-void kmod_builtin_iter_free(struct kmod_builtin_iter *iter) __attribute__((nonnull(1)));
-bool kmod_builtin_iter_next(struct kmod_builtin_iter *iter) __attribute__((nonnull(1)));
-bool kmod_builtin_iter_get_modname(struct kmod_builtin_iter *iter, char modname[static PATH_MAX]) __attribute__((nonnull(1, 2)));
 ssize_t kmod_builtin_get_modinfo(struct kmod_ctx *ctx, const char *modname, char ***modinfo) __attribute__((nonnull(1, 2, 3)));
-- 
2.39.2

[PATCH 3/4] shared: annotate local API as static

[Emil Velikov]

From: Emil Velikov <emil.velikov@collabora.com>

None of the API is used outside of the compilation unit.

Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
 shared/util.c | 6 +++---
 shared/util.h | 3 ---
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/shared/util.c b/shared/util.c
index 4b547ff..e2bab83 100644
--- a/shared/util.c
+++ b/shared/util.c
@@ -354,7 +354,7 @@ char *freadline_wrapped(FILE *fp, unsigned int *linenum)
 /* path handling functions                                                  */
 /* ************************************************************************ */
 
-bool path_is_absolute(const char *p)
+static bool path_is_absolute(const char *p)
 {
 	assert(p != NULL);
 
@@ -460,13 +460,13 @@ int mkdir_parents(const char *path, mode_t mode)
 	return mkdir_p(path, end - path, mode);
 }
 
-unsigned long long ts_usec(const struct timespec *ts)
+static unsigned long long ts_usec(const struct timespec *ts)
 {
 	return (unsigned long long) ts->tv_sec * USEC_PER_SEC +
 	       (unsigned long long) ts->tv_nsec / NSEC_PER_USEC;
 }
 
-unsigned long long ts_msec(const struct timespec *ts)
+static unsigned long long ts_msec(const struct timespec *ts)
 {
 	return (unsigned long long) ts->tv_sec * MSEC_PER_SEC +
 	       (unsigned long long) ts->tv_nsec / NSEC_PER_MSEC;
diff --git a/shared/util.h b/shared/util.h
index 7030653..c4a3916 100644
--- a/shared/util.h
+++ b/shared/util.h
@@ -38,7 +38,6 @@ char *freadline_wrapped(FILE *fp, unsigned int *linenum) __attribute__((nonnull(
 
 /* path handling functions                                                  */
 /* ************************************************************************ */
-bool path_is_absolute(const char *p) _must_check_ __attribute__((nonnull(1)));
 char *path_make_absolute_cwd(const char *p) _must_check_ __attribute__((nonnull(1)));
 int mkdir_p(const char *path, int len, mode_t mode);
 int mkdir_parents(const char *path, mode_t mode);
@@ -51,8 +50,6 @@ unsigned long long stat_mstamp(const struct stat *st);
 #define MSEC_PER_SEC	1000ULL
 #define NSEC_PER_MSEC	1000000ULL
 
-unsigned long long ts_usec(const struct timespec *ts);
-unsigned long long ts_msec(const struct timespec *ts);
 unsigned long long now_usec(void);
 unsigned long long now_msec(void);
 int sleep_until_msec(unsigned long long msec);
-- 
2.39.2

[PATCH 4/4] configure: manage libkmod.pc.in and version.py.in via AC_CONFIG_FILES

[Emil Velikov]

From: Emil Velikov <emil.velikov@collabora.com>

Replace the manual sed command, build rules and dist/clean for using
AC_CONFIG_FILES. It does the exact same thing, with an added bonus...

Currently we're missing version.py.in in the EXTRA_DIST. Thus a simple
"touch Makefile" should retrigger the regeneration of version.py. Which
would presumably fail, since the input file isn't in the distribution
tarball.

Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
Feel free to compare the diff between the old/new "make distcheck".
Cannot realistically add that here as it seemingly confuses the hell out
of git am.
---
 Makefile.am  | 25 -------------------------
 configure.ac |  2 ++
 2 files changed, 2 insertions(+), 25 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 61dbdf0..8ba85c9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -24,26 +24,6 @@ AM_CPPFLAGS = \
 AM_CFLAGS = $(OUR_CFLAGS)
 AM_LDFLAGS = $(OUR_LDFLAGS)
 
-SED_PROCESS = \
-	$(AM_V_GEN)$(MKDIR_P) $(dir $@) && $(SED) \
-	-e 's,@VERSION\@,$(VERSION),g' \
-	-e 's,@prefix\@,$(prefix),g' \
-	-e 's,@exec_prefix\@,$(exec_prefix),g' \
-	-e 's,@libdir\@,$(libdir),g' \
-	-e 's,@includedir\@,$(includedir),g' \
-	-e 's,@libzstd_CFLAGS\@,${libzstd_CFLAGS},g' \
-	-e 's,@libzstd_LIBS\@,${libzstd_LIBS},g' \
-	-e 's,@liblzma_CFLAGS\@,${liblzma_CFLAGS},g' \
-	-e 's,@liblzma_LIBS\@,${liblzma_LIBS},g' \
-	-e 's,@zlib_CFLAGS\@,${zlib_CFLAGS},g' \
-	-e 's,@zlib_LIBS\@,${zlib_LIBS},g' \
-	-e 's,@libcrypto_CFLAGS\@,${libcrypto_CFLAGS},g' \
-	-e 's,@libcrypto_LIBS\@,${libcrypto_LIBS},g' \
-	< $< > $@ || rm $@
-
-%.pc: %.pc.in Makefile
-	$(SED_PROCESS)
-
 # Rules for libtool versioning (from https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html)
 # 1. Start with version information of ‘0:0:0’ for each libtool library.
 # 2. Update the version information only immediately before a public release of
@@ -116,8 +96,6 @@ libkmod_libkmod_internal_la_LIBADD = $(libkmod_libkmod_la_LIBADD)
 
 pkgconfigdir = $(libdir)/pkgconfig
 pkgconfig_DATA = libkmod/libkmod.pc
-EXTRA_DIST += libkmod/libkmod.pc.in
-CLEANFILES += libkmod/libkmod.pc
 
 bashcompletiondir=@bashcompletiondir@
 dist_bashcompletion_DATA = \
@@ -179,9 +157,6 @@ am__v_CYTHON_0 = @echo "  CYTHON " $@;
 .pyx.c:
 	$(AM_V_CYTHON)$(CYTHON) -o $@ $<
 
-%.py: %.py.in Makefile
-	$(SED_PROCESS)
-
 # Remove some warnings for generated code
 PYTHON_NOWARN = -Wno-redundant-decls -Wno-shadow -Wno-strict-aliasing
 
diff --git a/configure.ac b/configure.ac
index 892f5d9..65902d1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -292,6 +292,8 @@ AC_CONFIG_FILES([
 	man/Makefile
 	libkmod/docs/Makefile
 	libkmod/docs/version.xml
+	libkmod/libkmod.pc
+	libkmod/python/kmod/version.py
 ])
 
 
-- 
2.39.2

Re: [PATCH 0/4] kmod: Paper cuts - dead code removal, don't SED manually

[Lucas De Marchi]

On Tue, 21 Feb 2023 13:19:25 +0000, Emil Velikov wrote:
> Here's another, final, round of paper cuts while browsing through the project.
> 
> Note that despite the enabled linker garbage collector, the first few cleanup
> patches result in detectable improvement in the final binaries.
> 
> The last patch, removes the open-coded replacement and by doing so fixes a
> genuine bug - albeit one that is uncommon to hit.
> 
> [...]

Applied all the patches, thanks!

[1/4] libkmod: remove unused kmod_module_get_builtin
      commit: 0237665beff4fa5e45b1d1ac5857627f949721b5
[2/4] libkmod: annotate kmod_builtin_iter API as static
      commit: df9d07a1492d7185413985add42ab38650ec2378
[3/4] shared: annotate local API as static
      commit: 06e6f167c211106212290aa7980880f972d71ba2
[4/4] configure: manage libkmod.pc.in and version.py.in via AC_CONFIG_FILES
      commit: e4c1a5b2998bc2c9dbcff8d62f121d1f8f5f4fe5

My intention is to release kmod 31 soon. Let me know if you have
anything pending that you'd like in the next release.  If nothing is pending I may just
do a release in the next days/week.


Best regards,
-- 
Lucas De Marchi <lucas.de.marchi@gmail.com>

Re: [PATCH 0/4] kmod: Paper cuts - dead code removal, don't SED manually

[Emil Velikov]

Greetings Lucas,

On Wed, 22 Feb 2023 at 00:40, Lucas De Marchi <lucas.de.marchi@gmail.com> wrote:

> My intention is to release kmod 31 soon. Let me know if you have
> anything pending that you'd like in the next release.  If nothing is pending I may just
> do a release in the next days/week.
>

There are no other patches on my end - rolling a release would be great.

Thanks o/
Emil