linux-modules.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lucas De Marchi <lucas.demarchi@intel.com>
To: <emil.l.velikov@gmail.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 12:39:03 -0500	[thread overview]
Message-ID: <37jvg2krroliuncyposassjhznjcgrxmvdre4mesl23npukbpn@jz7ntjkcpd7o> (raw)
In-Reply-To: <20240212-decompression-fixes-v1-9-06f92ad07985@gmail.com>

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]
    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

  parent reply	other threads:[~2024-04-30 17:39 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-12 17:23 [PATCH kmod 00/13] Load compressed modules with compression-less kmod Emil Velikov via B4 Relay
2024-02-12 17:23 ` [PATCH kmod 01/13] libkmod: use a dup()'d fd for zlib Emil Velikov via B4 Relay
2024-04-29 23:13   ` Lucas De Marchi
2024-02-12 17:23 ` [PATCH kmod 02/13] libkmod: keep gzFile gzf local to load_zlib() Emil Velikov via B4 Relay
2024-04-29 21:52   ` Lucas De Marchi
2024-02-12 17:23 ` [PATCH kmod 03/13] libkmod: remove kmod_file::{zstd,xz}_used flags Emil Velikov via B4 Relay
2024-04-29 21:54   ` Lucas De Marchi
2024-02-12 17:23 ` [PATCH kmod 04/13] libkmod: clear file->memory if map fails Emil Velikov via B4 Relay
2024-04-29 23:13   ` Lucas De Marchi
2024-02-12 17:23 ` [PATCH kmod 05/13] libkmod: nuke struct file_ops Emil Velikov via B4 Relay
2024-04-29 23:13   ` Lucas De Marchi
2024-02-12 17:23 ` [PATCH kmod 06/13] libkmod: propagate {zstd,xz,zlib}_load errors Emil Velikov via B4 Relay
2024-04-29 23:14   ` Lucas De Marchi
2024-02-12 17:23 ` [PATCH kmod 07/13] libkmod: move kmod_file_load_contents as applicable Emil Velikov via B4 Relay
2024-04-29 23:14   ` Lucas De Marchi
2024-02-12 17:23 ` [PATCH kmod 08/13] libkmod: always detect the module compression Emil Velikov via B4 Relay
2024-02-13 16:33   ` Emil Velikov
2024-04-29 23:13   ` Lucas De Marchi
2024-02-12 17:23 ` [PATCH kmod 09/13] libkmod: swap alloca usage for a few assert_cc Emil Velikov via B4 Relay
2024-04-29 23:19   ` Lucas De Marchi
2024-04-30 17:39   ` Lucas De Marchi [this message]
2024-04-30 17:54     ` Emil Velikov
2024-04-30 18:17       ` Lucas De Marchi
2024-04-30 18:27         ` Emil Velikov
2024-04-30 18:43           ` Lucas De Marchi
2024-04-30 18:47             ` Emil Velikov
2024-04-30 20:36               ` Lucas De Marchi
2024-02-12 17:23 ` [PATCH kmod 10/13] libkmod: tidy-up kmod_file_open() Emil Velikov via B4 Relay
2024-04-29 23:25   ` Lucas De Marchi
2024-02-12 17:23 ` [PATCH kmod 11/13] libkmod: move load_reg() further up Emil Velikov via B4 Relay
2024-04-29 23:30   ` Lucas De Marchi
2024-02-12 17:23 ` [PATCH kmod 12/13] libkmod: keep KMOD_FILE_COMPRESSION_NONE/load_reg in comp_types Emil Velikov via B4 Relay
2024-04-29 23:32   ` Lucas De Marchi
2024-02-12 17:23 ` [PATCH kmod 13/13] libkmod: always fallback to do_init_module() Emil Velikov via B4 Relay
2024-04-29 23:39   ` Lucas De Marchi
2024-04-30 17:48     ` Emil Velikov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=37jvg2krroliuncyposassjhznjcgrxmvdre4mesl23npukbpn@jz7ntjkcpd7o \
    --to=lucas.demarchi@intel.com \
    --cc=emil.l.velikov@gmail.com \
    --cc=linux-modules@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).