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.da...@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.ben...@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é <f4...@amsat.org>

Reply via email to