qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 7/8] semihosting: clean up handling of expanded argv


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v1 7/8] semihosting: clean up handling of expanded argv
Date: Mon, 21 Mar 2022 22:55:00 +0100
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.7.0

On 15/3/22 15:15, Daniel P. Berrangé wrote:
On Tue, Mar 15, 2022 at 01:59:59PM +0000, Alex Bennée wrote:

Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com> writes:

On 15/3/22 13:12, Alex Bennée wrote:
Another cleanup patch tripped over the fact we weren't being careful
in our casting. Fix the casts, allow for a non-const and switch from
g_realloc to g_renew.
The whole semihosting argument handling could do with some tests
though.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
   semihosting/config.c | 6 +++---
   1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/semihosting/config.c b/semihosting/config.c
index 137171b717..50d82108e6 100644
--- a/semihosting/config.c
+++ b/semihosting/config.c
@@ -51,7 +51,7 @@ typedef struct SemihostingConfig {
       bool enabled;
       SemihostingTarget target;
       Chardev *chardev;
-    const char **argv;
+    char **argv;
       int argc;
       const char *cmdline; /* concatenated argv */
   } SemihostingConfig;
@@ -98,8 +98,8 @@ static int add_semihosting_arg(void *opaque,
       if (strcmp(name, "arg") == 0) {
           s->argc++;
           /* one extra element as g_strjoinv() expects NULL-terminated array */
-        s->argv = g_realloc(s->argv, (s->argc + 1) * sizeof(void *));
-        s->argv[s->argc - 1] = val;
+        s->argv = g_renew(char *, s->argv, s->argc + 1);
+        s->argv[s->argc - 1] = g_strdup(val);

Why strdup()?

The compiler was having issues with adding a const char * into the array
and it was the quickest way to stop it complaining. I'm not sure what
guarantees you can make about a const char * after you leave the scope
of the function.

No guarantees at all. This method was implicitly relying on the caller
never free'ing the const arg it passed in. That is indeed the case here,
because the arg came from a QemuOpts list. It is bad practice to rely
on such things though, so adding the strdup is sane IMHO.

I see, thanks.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]