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

From: Emil Velikov <emil.l.velikov@gmail.com>
To: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: linux-modules@vger.kernel.org
Subject: Re: [PATCH kmod 09/13] libkmod: swap alloca usage for a few assert_cc
Date: Tue, 30 Apr 2024 18:54:00 +0100	[thread overview]
Message-ID: <CACvgo52PTWuDcv-P0Ls1oESo-cOsygUpD-rbK9oUTHy30HpS6w@mail.gmail.com> (raw)
In-Reply-To: <37jvg2krroliuncyposassjhznjcgrxmvdre4mesl23npukbpn@jz7ntjkcpd7o>

On Tue, 30 Apr 2024 at 18:39, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>
> On Mon, Feb 12, 2024 at 05:23:10PM GMT, Emil Velikov via B4 Relay wrote:
> >From: Emil Velikov <emil.l.velikov@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.velikov@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]

Is there a particular use-case for explicitly forcing C90?

The configure.ac contains `AC_PROG_CC_C99`, which seems reasonable
IMHO. Plus the autogen.sh goes a step further with `-std=gnu11`

Thanks
Emil