* [PATCH v2 1/9] audio/alsaaudio: Replace ALSA alloca() by malloc() equivalent
2021-05-06 13:37 [PATCH v2 0/9] misc: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
@ 2021-05-06 13:37 ` Philippe Mathieu-Daudé
2021-05-06 13:37 ` [PATCH v2 2/9] backends/tpm: Replace qemu_mutex_lock calls with QEMU_LOCK_GUARD Philippe Mathieu-Daudé
` (8 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-06 13:37 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, qemu-ppc, qemu-arm, Gerd Hoffmann, Paolo Bonzini,
Alex Bennée, Philippe Mathieu-Daudé
The ALLOCA(3) man-page mentions its "use is discouraged".
Define the cleanup functions for the snd_pcm_[hw/sw]_params_t types,
and replace the ALSA alloca() calls by equivalent ALSA malloc().
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
audio/alsaaudio.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index fcc2f62864f..f39061ebc42 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -70,6 +70,9 @@ struct alsa_params_obt {
snd_pcm_uframes_t samples;
};
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(snd_pcm_hw_params_t, snd_pcm_hw_params_free)
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(snd_pcm_sw_params_t, snd_pcm_sw_params_free)
+
static void GCC_FMT_ATTR (2, 3) alsa_logerr (int err, const char *fmt, ...)
{
va_list ap;
@@ -410,9 +413,9 @@ static void alsa_dump_info (struct alsa_params_req *req,
static void alsa_set_threshold (snd_pcm_t *handle, snd_pcm_uframes_t threshold)
{
int err;
- snd_pcm_sw_params_t *sw_params;
+ g_autoptr(snd_pcm_sw_params_t) sw_params = NULL;
- snd_pcm_sw_params_alloca (&sw_params);
+ snd_pcm_sw_params_malloc(&sw_params);
err = snd_pcm_sw_params_current (handle, sw_params);
if (err < 0) {
@@ -444,7 +447,7 @@ static int alsa_open(bool in, struct alsa_params_req *req,
AudiodevAlsaOptions *aopts = &dev->u.alsa;
AudiodevAlsaPerDirectionOptions *apdo = in ? aopts->in : aopts->out;
snd_pcm_t *handle;
- snd_pcm_hw_params_t *hw_params;
+ g_autoptr(snd_pcm_hw_params_t) hw_params = NULL;
int err;
unsigned int freq, nchannels;
const char *pcm_name = apdo->has_dev ? apdo->dev : "default";
@@ -455,7 +458,7 @@ static int alsa_open(bool in, struct alsa_params_req *req,
freq = req->freq;
nchannels = req->nchannels;
- snd_pcm_hw_params_alloca (&hw_params);
+ snd_pcm_hw_params_malloc(&hw_params);
err = snd_pcm_open (
&handle,
--
2.26.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 2/9] backends/tpm: Replace qemu_mutex_lock calls with QEMU_LOCK_GUARD
2021-05-06 13:37 [PATCH v2 0/9] misc: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
2021-05-06 13:37 ` [PATCH v2 1/9] audio/alsaaudio: Replace ALSA alloca() by malloc() equivalent Philippe Mathieu-Daudé
@ 2021-05-06 13:37 ` Philippe Mathieu-Daudé
2021-05-06 14:46 ` Stefan Berger
2021-05-06 15:07 ` Christophe de Dinechin
2021-05-06 13:37 ` [PATCH v2 3/9] backends/tpm: Replace g_alloca() by g_malloc() Philippe Mathieu-Daudé
` (7 subsequent siblings)
9 siblings, 2 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-06 13:37 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, qemu-ppc, qemu-arm, Gerd Hoffmann, Paolo Bonzini,
Alex Bennée, Philippe Mathieu-Daudé,
Stefan Berger
Simplify the tpm_emulator_ctrlcmd() handler by replacing a pair of
qemu_mutex_lock/qemu_mutex_unlock calls by the WITH_QEMU_LOCK_GUARD
macro.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
backends/tpm/tpm_emulator.c | 34 +++++++++++++++-------------------
1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index a012adc1934..e5f1063ab6c 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -30,6 +30,7 @@
#include "qemu/error-report.h"
#include "qemu/module.h"
#include "qemu/sockets.h"
+#include "qemu/lockable.h"
#include "io/channel-socket.h"
#include "sysemu/tpm_backend.h"
#include "sysemu/tpm_util.h"
@@ -124,31 +125,26 @@ static int tpm_emulator_ctrlcmd(TPMEmulator *tpm, unsigned long cmd, void *msg,
uint32_t cmd_no = cpu_to_be32(cmd);
ssize_t n = sizeof(uint32_t) + msg_len_in;
uint8_t *buf = NULL;
- int ret = -1;
- qemu_mutex_lock(&tpm->mutex);
+ WITH_QEMU_LOCK_GUARD(&tpm->mutex) {
+ buf = g_alloca(n);
+ memcpy(buf, &cmd_no, sizeof(cmd_no));
+ memcpy(buf + sizeof(cmd_no), msg, msg_len_in);
- buf = g_alloca(n);
- memcpy(buf, &cmd_no, sizeof(cmd_no));
- memcpy(buf + sizeof(cmd_no), msg, msg_len_in);
-
- n = qemu_chr_fe_write_all(dev, buf, n);
- if (n <= 0) {
- goto end;
- }
-
- if (msg_len_out != 0) {
- n = qemu_chr_fe_read_all(dev, msg, msg_len_out);
+ n = qemu_chr_fe_write_all(dev, buf, n);
if (n <= 0) {
- goto end;
+ return -1;
+ }
+
+ if (msg_len_out != 0) {
+ n = qemu_chr_fe_read_all(dev, msg, msg_len_out);
+ if (n <= 0) {
+ return -1;
+ }
}
}
- ret = 0;
-
-end:
- qemu_mutex_unlock(&tpm->mutex);
- return ret;
+ return 0;
}
static int tpm_emulator_unix_tx_bufs(TPMEmulator *tpm_emu,
--
2.26.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/9] backends/tpm: Replace qemu_mutex_lock calls with QEMU_LOCK_GUARD
2021-05-06 13:37 ` [PATCH v2 2/9] backends/tpm: Replace qemu_mutex_lock calls with QEMU_LOCK_GUARD Philippe Mathieu-Daudé
@ 2021-05-06 14:46 ` Stefan Berger
2021-05-06 15:07 ` Christophe de Dinechin
1 sibling, 0 replies; 25+ messages in thread
From: Stefan Berger @ 2021-05-06 14:46 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: kvm, Stefan Berger, qemu-arm, qemu-ppc, Gerd Hoffmann,
Paolo Bonzini, Alex Bennée
On 5/6/21 9:37 AM, Philippe Mathieu-Daudé wrote:
> Simplify the tpm_emulator_ctrlcmd() handler by replacing a pair of
> qemu_mutex_lock/qemu_mutex_unlock calls by the WITH_QEMU_LOCK_GUARD
> macro.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/9] backends/tpm: Replace qemu_mutex_lock calls with QEMU_LOCK_GUARD
2021-05-06 13:37 ` [PATCH v2 2/9] backends/tpm: Replace qemu_mutex_lock calls with QEMU_LOCK_GUARD Philippe Mathieu-Daudé
2021-05-06 14:46 ` Stefan Berger
@ 2021-05-06 15:07 ` Christophe de Dinechin
1 sibling, 0 replies; 25+ messages in thread
From: Christophe de Dinechin @ 2021-05-06 15:07 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, kvm, qemu-ppc, qemu-arm, Gerd Hoffmann,
Paolo Bonzini, Alex Bennée, Stefan Berger
On 2021-05-06 at 15:37 CEST, Philippe Mathieu-Daudé wrote...
> Simplify the tpm_emulator_ctrlcmd() handler by replacing a pair of
> qemu_mutex_lock/qemu_mutex_unlock calls by the WITH_QEMU_LOCK_GUARD
> macro.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> backends/tpm/tpm_emulator.c | 34 +++++++++++++++-------------------
> 1 file changed, 15 insertions(+), 19 deletions(-)
>
> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
> index a012adc1934..e5f1063ab6c 100644
> --- a/backends/tpm/tpm_emulator.c
> +++ b/backends/tpm/tpm_emulator.c
> @@ -30,6 +30,7 @@
> #include "qemu/error-report.h"
> #include "qemu/module.h"
> #include "qemu/sockets.h"
> +#include "qemu/lockable.h"
> #include "io/channel-socket.h"
> #include "sysemu/tpm_backend.h"
> #include "sysemu/tpm_util.h"
> @@ -124,31 +125,26 @@ static int tpm_emulator_ctrlcmd(TPMEmulator *tpm, unsigned long cmd, void *msg,
> uint32_t cmd_no = cpu_to_be32(cmd);
> ssize_t n = sizeof(uint32_t) + msg_len_in;
> uint8_t *buf = NULL;
> - int ret = -1;
>
> - qemu_mutex_lock(&tpm->mutex);
> + WITH_QEMU_LOCK_GUARD(&tpm->mutex) {
> + buf = g_alloca(n);
> + memcpy(buf, &cmd_no, sizeof(cmd_no));
> + memcpy(buf + sizeof(cmd_no), msg, msg_len_in);
>
> - buf = g_alloca(n);
> - memcpy(buf, &cmd_no, sizeof(cmd_no));
> - memcpy(buf + sizeof(cmd_no), msg, msg_len_in);
> -
> - n = qemu_chr_fe_write_all(dev, buf, n);
> - if (n <= 0) {
> - goto end;
> - }
> -
> - if (msg_len_out != 0) {
> - n = qemu_chr_fe_read_all(dev, msg, msg_len_out);
> + n = qemu_chr_fe_write_all(dev, buf, n);
> if (n <= 0) {
> - goto end;
> + return -1;
> + }
> +
> + if (msg_len_out != 0) {
> + n = qemu_chr_fe_read_all(dev, msg, msg_len_out);
> + if (n <= 0) {
> + return -1;
> + }
> }
> }
>
> - ret = 0;
> -
> -end:
> - qemu_mutex_unlock(&tpm->mutex);
> - return ret;
> + return 0;
> }
>
> static int tpm_emulator_unix_tx_bufs(TPMEmulator *tpm_emu,
I really like the improvement, but it looks like it does not belong to the
top-level series (i.e. not related to replacing alloca() by g_malloc()).
Reviewed-by: Christophe de Dinechin <dinechin@redhat.com>
--
Cheers,
Christophe de Dinechin (IRC c3d)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 3/9] backends/tpm: Replace g_alloca() by g_malloc()
2021-05-06 13:37 [PATCH v2 0/9] misc: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
2021-05-06 13:37 ` [PATCH v2 1/9] audio/alsaaudio: Replace ALSA alloca() by malloc() equivalent Philippe Mathieu-Daudé
2021-05-06 13:37 ` [PATCH v2 2/9] backends/tpm: Replace qemu_mutex_lock calls with QEMU_LOCK_GUARD Philippe Mathieu-Daudé
@ 2021-05-06 13:37 ` Philippe Mathieu-Daudé
2021-05-06 14:46 ` Stefan Berger
2021-05-06 13:37 ` [PATCH v2 4/9] bsd-user/syscall: Replace alloca() by g_new() Philippe Mathieu-Daudé
` (6 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-06 13:37 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, qemu-ppc, qemu-arm, Gerd Hoffmann, Paolo Bonzini,
Alex Bennée, Philippe Mathieu-Daudé,
Stefan Berger
The ALLOCA(3) man-page mentions its "use is discouraged".
Replace a g_alloca() call by a g_malloc() one, moving the
allocation before the MUTEX guarded block.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
backends/tpm/tpm_emulator.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index e5f1063ab6c..d37a6d563a3 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -124,10 +124,9 @@ static int tpm_emulator_ctrlcmd(TPMEmulator *tpm, unsigned long cmd, void *msg,
CharBackend *dev = &tpm->ctrl_chr;
uint32_t cmd_no = cpu_to_be32(cmd);
ssize_t n = sizeof(uint32_t) + msg_len_in;
- uint8_t *buf = NULL;
+ g_autofree uint8_t *buf = g_malloc(n);
WITH_QEMU_LOCK_GUARD(&tpm->mutex) {
- buf = g_alloca(n);
memcpy(buf, &cmd_no, sizeof(cmd_no));
memcpy(buf + sizeof(cmd_no), msg, msg_len_in);
--
2.26.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2 3/9] backends/tpm: Replace g_alloca() by g_malloc()
2021-05-06 13:37 ` [PATCH v2 3/9] backends/tpm: Replace g_alloca() by g_malloc() Philippe Mathieu-Daudé
@ 2021-05-06 14:46 ` Stefan Berger
0 siblings, 0 replies; 25+ messages in thread
From: Stefan Berger @ 2021-05-06 14:46 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: kvm, qemu-ppc, qemu-arm, Gerd Hoffmann, Paolo Bonzini,
Alex Bennée, Stefan Berger
On 5/6/21 9:37 AM, Philippe Mathieu-Daudé wrote:
> The ALLOCA(3) man-page mentions its "use is discouraged".
>
> Replace a g_alloca() call by a g_malloc() one, moving the
> allocation before the MUTEX guarded block.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 4/9] bsd-user/syscall: Replace alloca() by g_new()
2021-05-06 13:37 [PATCH v2 0/9] misc: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2021-05-06 13:37 ` [PATCH v2 3/9] backends/tpm: Replace g_alloca() by g_malloc() Philippe Mathieu-Daudé
@ 2021-05-06 13:37 ` Philippe Mathieu-Daudé
[not found] ` <CANCZdfoJWEbPFvZ0605riUfnpVRAeC6Feem5_ahC7FUfO71-AA@mail.gmail.com>
2021-05-06 13:37 ` [PATCH v2 5/9] gdbstub: Constify GdbCmdParseEntry Philippe Mathieu-Daudé
` (5 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-06 13:37 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, qemu-ppc, qemu-arm, Gerd Hoffmann, Paolo Bonzini,
Alex Bennée, Philippe Mathieu-Daudé,
Warner Losh, Kyle Evans
The ALLOCA(3) man-page mentions its "use is discouraged".
Replace it by a g_new() call.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
bsd-user/syscall.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/bsd-user/syscall.c b/bsd-user/syscall.c
index 4abff796c76..dbee0385ceb 100644
--- a/bsd-user/syscall.c
+++ b/bsd-user/syscall.c
@@ -355,9 +355,8 @@ abi_long do_freebsd_syscall(void *cpu_env, int num, abi_long arg1,
case TARGET_FREEBSD_NR_writev:
{
int count = arg3;
- struct iovec *vec;
+ g_autofree struct iovec *vec = g_new(struct iovec, count);
- vec = alloca(count * sizeof(struct iovec));
if (lock_iovec(VERIFY_READ, vec, arg2, count, 1) < 0)
goto efault;
ret = get_errno(writev(arg1, vec, count));
--
2.26.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 5/9] gdbstub: Constify GdbCmdParseEntry
2021-05-06 13:37 [PATCH v2 0/9] misc: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2021-05-06 13:37 ` [PATCH v2 4/9] bsd-user/syscall: Replace alloca() by g_new() Philippe Mathieu-Daudé
@ 2021-05-06 13:37 ` Philippe Mathieu-Daudé
2021-05-06 13:37 ` [PATCH v2 6/9] gdbstub: Only call cmd_parse_params() with non-NULL command schema Philippe Mathieu-Daudé
` (4 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-06 13:37 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, qemu-ppc, qemu-arm, Gerd Hoffmann, Paolo Bonzini,
Alex Bennée, Philippe Mathieu-Daudé
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
gdbstub.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 9103ffc9028..83d47c67325 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1981,7 +1981,7 @@ static void handle_v_kill(GdbCmdContext *gdb_ctx, void *user_ctx)
exit(0);
}
-static GdbCmdParseEntry gdb_v_commands_table[] = {
+static const GdbCmdParseEntry gdb_v_commands_table[] = {
/* Order is important if has same prefix */
{
.handler = handle_v_cont_query,
@@ -2324,7 +2324,7 @@ static void handle_set_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx, void *user_ctx)
}
#endif
-static GdbCmdParseEntry gdb_gen_query_set_common_table[] = {
+static const GdbCmdParseEntry gdb_gen_query_set_common_table[] = {
/* Order is important if has same prefix */
{
.handler = handle_query_qemu_sstepbits,
@@ -2342,7 +2342,7 @@ static GdbCmdParseEntry gdb_gen_query_set_common_table[] = {
},
};
-static GdbCmdParseEntry gdb_gen_query_table[] = {
+static const GdbCmdParseEntry gdb_gen_query_table[] = {
{
.handler = handle_query_curr_tid,
.cmd = "C",
@@ -2420,7 +2420,7 @@ static GdbCmdParseEntry gdb_gen_query_table[] = {
#endif
};
-static GdbCmdParseEntry gdb_gen_set_table[] = {
+static const GdbCmdParseEntry gdb_gen_set_table[] = {
/* Order is important if has same prefix */
{
.handler = handle_set_qemu_sstep,
--
2.26.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 6/9] gdbstub: Only call cmd_parse_params() with non-NULL command schema
2021-05-06 13:37 [PATCH v2 0/9] misc: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2021-05-06 13:37 ` [PATCH v2 5/9] gdbstub: Constify GdbCmdParseEntry Philippe Mathieu-Daudé
@ 2021-05-06 13:37 ` Philippe Mathieu-Daudé
2021-05-06 19:21 ` Alex Bennée
2021-05-06 13:37 ` [PATCH v2 7/9] gdbstub: Replace alloca() + memset(0) by g_new0() Philippe Mathieu-Daudé
` (3 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-06 13:37 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, qemu-ppc, qemu-arm, Gerd Hoffmann, Paolo Bonzini,
Alex Bennée, Philippe Mathieu-Daudé
Move the NULL check on command schema buffer from the callee
cmd_parse_params() to the single caller, process_string_cmd().
This simplifies the process_string_cmd() logic.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
gdbstub.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 83d47c67325..7cee2fb0f1f 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1368,12 +1368,9 @@ static int cmd_parse_params(const char *data, const char *schema,
int curr_param;
const char *curr_schema, *curr_data;
+ assert(schema);
*num_params = 0;
- if (!schema) {
- return 0;
- }
-
curr_schema = schema;
curr_param = 0;
curr_data = data;
@@ -1471,7 +1468,7 @@ static inline int startswith(const char *string, const char *pattern)
static int process_string_cmd(void *user_ctx, const char *data,
const GdbCmdParseEntry *cmds, int num_cmds)
{
- int i, schema_len, max_num_params = 0;
+ int i;
GdbCmdContext gdb_ctx;
if (!cmds) {
@@ -1488,21 +1485,21 @@ static int process_string_cmd(void *user_ctx, const char *data,
}
if (cmd->schema) {
- schema_len = strlen(cmd->schema);
+ int schema_len = strlen(cmd->schema);
+ int max_num_params = schema_len / 2;
+
if (schema_len % 2) {
return -2;
}
- max_num_params = schema_len / 2;
- }
+ gdb_ctx.params = (GdbCmdVariant *)alloca(sizeof(*gdb_ctx.params)
+ * max_num_params);
+ memset(gdb_ctx.params, 0, sizeof(*gdb_ctx.params) * max_num_params);
- gdb_ctx.params =
- (GdbCmdVariant *)alloca(sizeof(*gdb_ctx.params) * max_num_params);
- memset(gdb_ctx.params, 0, sizeof(*gdb_ctx.params) * max_num_params);
-
- if (cmd_parse_params(&data[strlen(cmd->cmd)], cmd->schema,
- gdb_ctx.params, &gdb_ctx.num_params)) {
- return -1;
+ if (cmd_parse_params(&data[strlen(cmd->cmd)], cmd->schema,
+ gdb_ctx.params, &gdb_ctx.num_params)) {
+ return -1;
+ }
}
cmd->handler(&gdb_ctx, user_ctx);
--
2.26.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 7/9] gdbstub: Replace alloca() + memset(0) by g_new0()
2021-05-06 13:37 [PATCH v2 0/9] misc: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2021-05-06 13:37 ` [PATCH v2 6/9] gdbstub: Only call cmd_parse_params() with non-NULL command schema Philippe Mathieu-Daudé
@ 2021-05-06 13:37 ` Philippe Mathieu-Daudé
2021-05-06 19:22 ` Alex Bennée
2021-05-06 13:37 ` [PATCH v2 8/9] hw/misc/pca9552: Replace g_newa() by g_new() Philippe Mathieu-Daudé
` (2 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-06 13:37 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, qemu-ppc, qemu-arm, Gerd Hoffmann, Paolo Bonzini,
Alex Bennée, Philippe Mathieu-Daudé
The ALLOCA(3) man-page mentions its "use is discouraged".
Replace the alloca() and memset(0) calls by g_new0().
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
gdbstub.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 7cee2fb0f1f..666053bf590 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1487,14 +1487,13 @@ static int process_string_cmd(void *user_ctx, const char *data,
if (cmd->schema) {
int schema_len = strlen(cmd->schema);
int max_num_params = schema_len / 2;
+ g_autofree GdbCmdVariant *params = NULL;
if (schema_len % 2) {
return -2;
}
- gdb_ctx.params = (GdbCmdVariant *)alloca(sizeof(*gdb_ctx.params)
- * max_num_params);
- memset(gdb_ctx.params, 0, sizeof(*gdb_ctx.params) * max_num_params);
+ gdb_ctx.params = params = g_new0(GdbCmdVariant, max_num_params);
if (cmd_parse_params(&data[strlen(cmd->cmd)], cmd->schema,
gdb_ctx.params, &gdb_ctx.num_params)) {
--
2.26.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2 7/9] gdbstub: Replace alloca() + memset(0) by g_new0()
2021-05-06 13:37 ` [PATCH v2 7/9] gdbstub: Replace alloca() + memset(0) by g_new0() Philippe Mathieu-Daudé
@ 2021-05-06 19:22 ` Alex Bennée
0 siblings, 0 replies; 25+ messages in thread
From: Alex Bennée @ 2021-05-06 19:22 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, kvm, qemu-ppc, qemu-arm, Gerd Hoffmann, Paolo Bonzini
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> The ALLOCA(3) man-page mentions its "use is discouraged".
>
> Replace the alloca() and memset(0) calls by g_new0().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Please see:
Subject: [ALT PATCH] gdbstub: Replace GdbCmdContext with plain g_array()
Date: Thu, 6 May 2021 17:07:41 +0100
Message-Id: <20210506160741.9841-1-alex.bennee@linaro.org>
which also includes elements of 6/9 which can be kept split off.
> ---
> gdbstub.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 7cee2fb0f1f..666053bf590 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1487,14 +1487,13 @@ static int process_string_cmd(void *user_ctx, const char *data,
> if (cmd->schema) {
> int schema_len = strlen(cmd->schema);
> int max_num_params = schema_len / 2;
> + g_autofree GdbCmdVariant *params = NULL;
>
> if (schema_len % 2) {
> return -2;
> }
>
> - gdb_ctx.params = (GdbCmdVariant *)alloca(sizeof(*gdb_ctx.params)
> - * max_num_params);
> - memset(gdb_ctx.params, 0, sizeof(*gdb_ctx.params) * max_num_params);
> + gdb_ctx.params = params = g_new0(GdbCmdVariant, max_num_params);
>
> if (cmd_parse_params(&data[strlen(cmd->cmd)], cmd->schema,
> gdb_ctx.params, &gdb_ctx.num_params)) {
--
Alex Bennée
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 8/9] hw/misc/pca9552: Replace g_newa() by g_new()
2021-05-06 13:37 [PATCH v2 0/9] misc: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2021-05-06 13:37 ` [PATCH v2 7/9] gdbstub: Replace alloca() + memset(0) by g_new0() Philippe Mathieu-Daudé
@ 2021-05-06 13:37 ` Philippe Mathieu-Daudé
2021-05-06 13:37 ` [PATCH v2 9/9] target/ppc/kvm: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
[not found] ` <CANCZdfqiHxQoG+g3bq_KL01yWCHUbF5qxJWN=sD37h7UJFMZ7g@mail.gmail.com>
9 siblings, 0 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-06 13:37 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, qemu-ppc, qemu-arm, Gerd Hoffmann, Paolo Bonzini,
Alex Bennée, Philippe Mathieu-Daudé,
Cédric Le Goater, Peter Maydell, Andrew Jeffery,
Joel Stanley
The ALLOCA(3) man-page mentions its "use is discouraged".
Replace the g_newa() call by g_new().
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/misc/pca9552.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
index b7686e27d7f..facf103cbfb 100644
--- a/hw/misc/pca9552.c
+++ b/hw/misc/pca9552.c
@@ -71,7 +71,7 @@ static void pca955x_display_pins_status(PCA955xState *s,
return;
}
if (trace_event_get_state_backends(TRACE_PCA955X_GPIO_STATUS)) {
- char *buf = g_newa(char, k->pin_count + 1);
+ g_autofree char *buf = g_new(char, k->pin_count + 1);
for (i = 0; i < k->pin_count; i++) {
if (extract32(pins_status, i, 1)) {
--
2.26.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 9/9] target/ppc/kvm: Replace alloca() by g_malloc()
2021-05-06 13:37 [PATCH v2 0/9] misc: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2021-05-06 13:37 ` [PATCH v2 8/9] hw/misc/pca9552: Replace g_newa() by g_new() Philippe Mathieu-Daudé
@ 2021-05-06 13:37 ` Philippe Mathieu-Daudé
2021-05-06 14:45 ` Greg Kurz
2021-05-07 1:05 ` David Gibson
[not found] ` <CANCZdfqiHxQoG+g3bq_KL01yWCHUbF5qxJWN=sD37h7UJFMZ7g@mail.gmail.com>
9 siblings, 2 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-06 13:37 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, qemu-ppc, qemu-arm, Gerd Hoffmann, Paolo Bonzini,
Alex Bennée, Philippe Mathieu-Daudé,
David Gibson, Greg Kurz
The ALLOCA(3) man-page mentions its "use is discouraged".
Replace it by a g_malloc() call.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
target/ppc/kvm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 104a308abb5..63c458e2211 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2698,11 +2698,10 @@ int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns)
int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
uint16_t n_valid, uint16_t n_invalid, Error **errp)
{
- struct kvm_get_htab_header *buf;
size_t chunksize = sizeof(*buf) + n_valid * HASH_PTE_SIZE_64;
+ g_autofree struct kvm_get_htab_header *buf = g_malloc(chunksize);
ssize_t rc;
- buf = alloca(chunksize);
buf->index = index;
buf->n_valid = n_valid;
buf->n_invalid = n_invalid;
--
2.26.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2 9/9] target/ppc/kvm: Replace alloca() by g_malloc()
2021-05-06 13:37 ` [PATCH v2 9/9] target/ppc/kvm: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
@ 2021-05-06 14:45 ` Greg Kurz
2021-05-07 1:05 ` David Gibson
1 sibling, 0 replies; 25+ messages in thread
From: Greg Kurz @ 2021-05-06 14:45 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, kvm, qemu-ppc, qemu-arm, Gerd Hoffmann,
Paolo Bonzini, Alex Bennée, David Gibson
On Thu, 6 May 2021 15:37:58 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> The ALLOCA(3) man-page mentions its "use is discouraged".
>
> Replace it by a g_malloc() call.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> target/ppc/kvm.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 104a308abb5..63c458e2211 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2698,11 +2698,10 @@ int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns)
> int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
> uint16_t n_valid, uint16_t n_invalid, Error **errp)
> {
> - struct kvm_get_htab_header *buf;
> size_t chunksize = sizeof(*buf) + n_valid * HASH_PTE_SIZE_64;
> + g_autofree struct kvm_get_htab_header *buf = g_malloc(chunksize);
> ssize_t rc;
>
> - buf = alloca(chunksize);
> buf->index = index;
> buf->n_valid = n_valid;
> buf->n_invalid = n_invalid;
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 9/9] target/ppc/kvm: Replace alloca() by g_malloc()
2021-05-06 13:37 ` [PATCH v2 9/9] target/ppc/kvm: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
2021-05-06 14:45 ` Greg Kurz
@ 2021-05-07 1:05 ` David Gibson
1 sibling, 0 replies; 25+ messages in thread
From: David Gibson @ 2021-05-07 1:05 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, kvm, qemu-ppc, qemu-arm, Gerd Hoffmann,
Paolo Bonzini, Alex Bennée, Greg Kurz
[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]
On Thu, May 06, 2021 at 03:37:58PM +0200, Philippe Mathieu-Daudé wrote:
> The ALLOCA(3) man-page mentions its "use is discouraged".
>
> Replace it by a g_malloc() call.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> target/ppc/kvm.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 104a308abb5..63c458e2211 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2698,11 +2698,10 @@ int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns)
> int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
> uint16_t n_valid, uint16_t n_invalid, Error **errp)
> {
> - struct kvm_get_htab_header *buf;
> size_t chunksize = sizeof(*buf) + n_valid * HASH_PTE_SIZE_64;
> + g_autofree struct kvm_get_htab_header *buf = g_malloc(chunksize);
Um.. that doesn't look like it would compile, since you use
sizeof(*buf) before declaring buf.
> ssize_t rc;
>
> - buf = alloca(chunksize);
> buf->index = index;
> buf->n_valid = n_valid;
> buf->n_invalid = n_invalid;
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
[parent not found: <CANCZdfqiHxQoG+g3bq_KL01yWCHUbF5qxJWN=sD37h7UJFMZ7g@mail.gmail.com>]
* Re: [PATCH v2 0/9] misc: Replace alloca() by g_malloc()
[not found] ` <CANCZdfqiHxQoG+g3bq_KL01yWCHUbF5qxJWN=sD37h7UJFMZ7g@mail.gmail.com>
@ 2021-05-06 14:28 ` Eric Blake
0 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2021-05-06 14:28 UTC (permalink / raw)
To: Warner Losh, Philippe Mathieu-Daudé
Cc: kvm, QEMU Developers, qemu-arm, qemu-ppc, Gerd Hoffmann,
Paolo Bonzini, Alex Bennée
On 5/6/21 9:22 AM, Warner Losh wrote:
> On Thu, May 6, 2021 at 7:39 AM Philippe Mathieu-Daudé <philmd@redhat.com>
> wrote:
>
>> The ALLOCA(3) man-page mentions its "use is discouraged".
>> Replace few calls by equivalent GLib malloc().
>>
>
> Except g_alloc and g_malloc are not at all the same, and you can't drop in
> replace one with the other.
>
> g_alloc allocates stack space on the calling frame that's automatically
> freed when the function returns.
> g_malloc allocates space from the heap, and calls to it must be matched
> with calls to g_free().
>
> These patches don't do the latter, as far as I can tell, and so introduce
> memory leaks unless there's something I've missed.
You missed the g_autofree, whose job is to call g_free() on all points
in the control flow where the malloc()d memory goes out of scope
(equivalent in expressive power to alloca()d memory going out of scope).
Although the code is arguably a bit slower (as heap manipulations are
not as cheap as stack manipulations), in the long run that speed penalty
is worth the safety factor (since stack manipulations under user control
are inherently unsafe).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 25+ messages in thread