| Message ID | 20250605193540.59874-4-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | system: Forbid alloca() | expand |
On 6/5/25 12:35 PM, Philippe Mathieu-Daudé wrote: > Do not use g_alloca(), simply allocate the CharBackend > structure on the stack. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > tests/unit/test-char.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c > index 60a843b79d9..f30a39f61ff 100644 > --- a/tests/unit/test-char.c > +++ b/tests/unit/test-char.c > @@ -993,7 +993,7 @@ static void char_udp_test_internal(Chardev *reuse_chr, int sock) > struct sockaddr_in other; > SocketIdleData d = { 0, }; > Chardev *chr; > - CharBackend *be; > + CharBackend stack_be, *be = &stack_be; > socklen_t alen = sizeof(other); > int ret; > char buf[10]; > @@ -1009,7 +1009,6 @@ static void char_udp_test_internal(Chardev *reuse_chr, int sock) > chr = qemu_chr_new("client", tmp, NULL); > g_assert_nonnull(chr); > > - be = g_alloca(sizeof(CharBackend)); > qemu_chr_fe_init(be, chr, &error_abort); > } > Would that be more simple to declare the variable, and use &be in the function code?
On 5/6/25 22:53, Pierrick Bouvier wrote: > On 6/5/25 12:35 PM, Philippe Mathieu-Daudé wrote: >> Do not use g_alloca(), simply allocate the CharBackend >> structure on the stack. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> tests/unit/test-char.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c >> index 60a843b79d9..f30a39f61ff 100644 >> --- a/tests/unit/test-char.c >> +++ b/tests/unit/test-char.c >> @@ -993,7 +993,7 @@ static void char_udp_test_internal(Chardev >> *reuse_chr, int sock) >> struct sockaddr_in other; >> SocketIdleData d = { 0, }; >> Chardev *chr; >> - CharBackend *be; >> + CharBackend stack_be, *be = &stack_be; >> socklen_t alen = sizeof(other); >> int ret; >> char buf[10]; >> @@ -1009,7 +1009,6 @@ static void char_udp_test_internal(Chardev >> *reuse_chr, int sock) >> chr = qemu_chr_new("client", tmp, NULL); >> g_assert_nonnull(chr); >> - be = g_alloca(sizeof(CharBackend)); >> qemu_chr_fe_init(be, chr, &error_abort); >> } > > Would that be more simple to declare the variable, and use &be in the > function code? More context (see reuse_chr ladder): -- >8 -- @@ -991,45 +991,44 @@ static int make_udp_socket(int *port) static void char_udp_test_internal(Chardev *reuse_chr, int sock) { struct sockaddr_in other; SocketIdleData d = { 0, }; Chardev *chr; - CharBackend *be; + CharBackend stack_be, *be = &stack_be; socklen_t alen = sizeof(other); int ret; char buf[10]; char *tmp = NULL; if (reuse_chr) { chr = reuse_chr; be = chr->be; } else { int port; sock = make_udp_socket(&port); tmp = g_strdup_printf("udp:127.0.0.1:%d", port); chr = qemu_chr_new("client", tmp, NULL); g_assert_nonnull(chr); - be = g_alloca(sizeof(CharBackend)); qemu_chr_fe_init(be, chr, &error_abort); } ---
On 6/5/25 11:09 PM, Philippe Mathieu-Daudé wrote: > On 5/6/25 22:53, Pierrick Bouvier wrote: >> On 6/5/25 12:35 PM, Philippe Mathieu-Daudé wrote: >>> Do not use g_alloca(), simply allocate the CharBackend >>> structure on the stack. >>> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>> --- >>> tests/unit/test-char.c | 3 +-- >>> 1 file changed, 1 insertion(+), 2 deletions(-) >>> >>> diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c >>> index 60a843b79d9..f30a39f61ff 100644 >>> --- a/tests/unit/test-char.c >>> +++ b/tests/unit/test-char.c >>> @@ -993,7 +993,7 @@ static void char_udp_test_internal(Chardev >>> *reuse_chr, int sock) >>> struct sockaddr_in other; >>> SocketIdleData d = { 0, }; >>> Chardev *chr; >>> - CharBackend *be; >>> + CharBackend stack_be, *be = &stack_be; >>> socklen_t alen = sizeof(other); >>> int ret; >>> char buf[10]; >>> @@ -1009,7 +1009,6 @@ static void char_udp_test_internal(Chardev >>> *reuse_chr, int sock) >>> chr = qemu_chr_new("client", tmp, NULL); >>> g_assert_nonnull(chr); >>> - be = g_alloca(sizeof(CharBackend)); >>> qemu_chr_fe_init(be, chr, &error_abort); >>> } >> >> Would that be more simple to declare the variable, and use &be in the >> function code? > > More context (see reuse_chr ladder): > > -- >8 -- > @@ -991,45 +991,44 @@ static int make_udp_socket(int *port) > static void char_udp_test_internal(Chardev *reuse_chr, int sock) > { > struct sockaddr_in other; > SocketIdleData d = { 0, }; > Chardev *chr; > - CharBackend *be; > + CharBackend stack_be, *be = &stack_be; > socklen_t alen = sizeof(other); > int ret; > char buf[10]; > char *tmp = NULL; > > if (reuse_chr) { > chr = reuse_chr; > be = chr->be; > } else { > int port; > sock = make_udp_socket(&port); > tmp = g_strdup_printf("udp:127.0.0.1:%d", port); > chr = qemu_chr_new("client", tmp, NULL); > g_assert_nonnull(chr); > > - be = g_alloca(sizeof(CharBackend)); > qemu_chr_fe_init(be, chr, &error_abort); > } > --- Ok!
diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c index 60a843b79d9..f30a39f61ff 100644 --- a/tests/unit/test-char.c +++ b/tests/unit/test-char.c @@ -993,7 +993,7 @@ static void char_udp_test_internal(Chardev *reuse_chr, int sock) struct sockaddr_in other; SocketIdleData d = { 0, }; Chardev *chr; - CharBackend *be; + CharBackend stack_be, *be = &stack_be; socklen_t alen = sizeof(other); int ret; char buf[10]; @@ -1009,7 +1009,6 @@ static void char_udp_test_internal(Chardev *reuse_chr, int sock) chr = qemu_chr_new("client", tmp, NULL); g_assert_nonnull(chr); - be = g_alloca(sizeof(CharBackend)); qemu_chr_fe_init(be, chr, &error_abort); }
Do not use g_alloca(), simply allocate the CharBackend structure on the stack. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- tests/unit/test-char.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)