Re: [PATCH kmod 09/13] libkmod: swap alloca usage for a few assert_cc

[Lucas De Marchi]

On Mon, Feb 12, 2024 at 05:23:10PM GMT, Emil Velikov via B4 Relay wrote:
From: Emil Velikov <emil.l.veli...@gmail.com>
Since all the compression magic is always available now, we don't need
to loop at runtime nor use alloca - latter of which comes with a handful
of caveats.

Simply throw in a few assert_cc(), which will trigger at build-time.

Signed-off-by: Emil Velikov <emil.l.veli...@gmail.com>
---
libkmod/libkmod-file.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/libkmod/libkmod-file.c b/libkmod/libkmod-file.c
index b69f1ef..5b88d6c 100644
--- a/libkmod/libkmod-file.c
+++ b/libkmod/libkmod-file.c
@@ -410,7 +410,6 @@ struct kmod_file *kmod_file_open(const struct kmod_ctx *ctx,
{
        struct kmod_file *file = calloc(1, sizeof(struct kmod_file));
        const struct comp_type *itr;
-       size_t magic_size_max = 0;
        int err = 0;

        if (file == NULL)
@@ -422,22 +421,17 @@ struct kmod_file *kmod_file_open(const struct kmod_ctx 
*ctx,
                goto error;
        }

-       for (itr = comp_types; itr->load != NULL; itr++) {
-               if (magic_size_max < itr->magic_size)
-                       magic_size_max = itr->magic_size;
-       }
-
-       if (magic_size_max > 0) {
-               char *buf = alloca(magic_size_max + 1);
+       {
+               char buf[7];
                ssize_t sz;

-               if (buf == NULL) {
-                       err = -errno;
-                       goto error;
-               }
-               sz = read_str_safe(file->fd, buf, magic_size_max + 1);
+               assert_cc(sizeof(magic_zstd) < sizeof(buf));
+               assert_cc(sizeof(magic_xz) < sizeof(buf));
+               assert_cc(sizeof(magic_zlib) < sizeof(buf));

../libkmod/libkmod-file.c: In function 'kmod_file_open':
../shared/macro.h:25:9: warning: ISO C90 forbids mixed declarations and code 
[-Wdeclaration-after-statement]
   25 |         _Static_assert((expr), #expr)
      |         ^~~~~~~~~~~~~~
../libkmod/libkmod-file.c:424:9: note: in expansion of macro 'assert_cc'
  424 |         assert_cc(sizeof(magic_zstd) < sizeof(buf));
      |         ^~~~~~~~~


So I'd go with this on top of this patch...  I can squash it as needed
when applying:


|diff --git a/libkmod/libkmod-file.c b/libkmod/libkmod-file.c
|index f162a10..8baf12d 100644
|--- a/libkmod/libkmod-file.c
|+++ b/libkmod/libkmod-file.c
|@@ -408,10 +408,15 @@ struct kmod_elf *kmod_file_get_elf(struct kmod_file 
*file)
| struct kmod_file *kmod_file_open(const struct kmod_ctx *ctx,
|                                               const char *filename)
| {
|-      struct kmod_file *file = calloc(1, sizeof(struct kmod_file));
|+      struct kmod_file *file;
|       char buf[7];
|       ssize_t sz;
| 
|+	assert_cc(sizeof(magic_zstd) < sizeof(buf));
|+      assert_cc(sizeof(magic_xz) < sizeof(buf));
|+      assert_cc(sizeof(magic_zlib) < sizeof(buf));
|+
|+      file = calloc(1, sizeof(struct kmod_file));
|       if (file == NULL)
|               return NULL;
| 
|@@ -421,10 +426,6 @@ struct kmod_file *kmod_file_open(const struct kmod_ctx *ctx,
|               return NULL;
|       }
| 
|-	assert_cc(sizeof(magic_zstd) < sizeof(buf));
|-      assert_cc(sizeof(magic_xz) < sizeof(buf));
|-      assert_cc(sizeof(magic_zlib) < sizeof(buf));
|-
|       sz = read_str_safe(file->fd, buf, sizeof(buf));
|       lseek(file->fd, 0, SEEK_SET);
|       if (sz != (sizeof(buf) - 1)) {

Lucas De Marchi