Re: [Qemu-devel] [PATCH 08/22] linux-user: remove two unchecked uses of strdup

[Jim Meyering]

Peter Maydell wrote:
> On 9 May 2012 10:23, Jim Meyering <address@hidden> wrote:
>> From: Jim Meyering <address@hidden>
>>
>> Remove unnecessary and unchecked uses of strdup,
>> and add a comment that this strncpy use is ok.
>>
>> Signed-off-by: Jim Meyering <address@hidden>
>> ---
>>  linux-user/elfload.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index f3b1552..25175cc 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -2338,12 +2338,14 @@ static int fill_psinfo(struct
>> target_elf_prpsinfo *psinfo, const TaskState *ts)
>>     psinfo->pr_uid = getuid();
>>     psinfo->pr_gid = getgid();
>>
>> -    filename = strdup(ts->bprm->filename);
>> -    base_filename = strdup(basename(filename));
>> +    filename = ts->bprm->filename;
>> +    base_filename = basename(filename);
>> +    /*
>> +     * Using strncpy here is fine: at max-length,
>> +     * this field is not NUL-terminated.
>> +     */
>>     (void) strncpy(psinfo->pr_fname, base_filename,
>>                    sizeof(psinfo->pr_fname));
>> -    free(base_filename);
>> -    free(filename);
>
> This doesn't look right -- basename can modify
> the string it's passed, which is why we create a copy
> for it.

Hi Peter,

Thank you for the review and the correction.
I'm spoiled from using gnulib with which basename never modifies its argument.

Here's an adjusted patch:

>From 5dce6a0222252cdc2a45ada3e3e96a8c3ef4e90f Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 7 May 2012 18:34:26 +0200
Subject: [PATCH] linux-user: remove two unchecked uses of strdup

Remove two uses of strdup (use g_path_get_basename instead),
and add a comment that this strncpy use is ok.

Signed-off-by: Jim Meyering <address@hidden>
---
 linux-user/elfload.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index f3b1552..977283e 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2338,13 +2338,16 @@ static int fill_psinfo(struct target_elf_prpsinfo 
*psinfo, const TaskState *ts)
     psinfo->pr_uid = getuid();
     psinfo->pr_gid = getgid();

-    filename = strdup(ts->bprm->filename);
-    base_filename = strdup(basename(filename));
+    filename = ts->bprm->filename;
+    base_filename = g_path_get_basename(filename);
+    /*
+     * Using strncpy here is fine: at max-length,
+     * this field is not NUL-terminated.
+     */
     (void) strncpy(psinfo->pr_fname, base_filename,
                    sizeof(psinfo->pr_fname));
-    free(base_filename);
-    free(filename);

+    free(base_filename);
     bswap_psinfo(psinfo);
     return (0);
 }
--
1.7.10.1.487.ga3935e6