diff mbox

[v2,03/11] hw/9pfs: Fix unchecked strdup() by converting to g_strdup()

Message ID 1358357540-29862-4-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster Jan. 16, 2013, 5:32 p.m. UTC 
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 hw/9pfs/virtio-9p-device.c | 2 +-
 hw/9pfs/virtio-9p-local.c  | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

Comments

Stefan Hajnoczi Jan. 17, 2013, 11:19 a.m. UTC | #1 
On Wed, Jan 16, 2013 at 06:32:12PM +0100, Markus Armbruster wrote:
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 6eab7f7..74155fb 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -94,7 +94,7 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
>          exit(1);
>      }
>  
> -    s->tag = strdup(conf->tag);
> +    s->tag = g_strdup(conf->tag);
>      s->ctx.uid = -1;
>  
>      s->ops = fse->ops;

s->tag is leaked.  Want to send a follow-up patch to g_free() it?
Markus Armbruster Jan. 17, 2013, 1:21 p.m. UTC | #2 
Stefan Hajnoczi <stefanha@gmail.com> writes:

> On Wed, Jan 16, 2013 at 06:32:12PM +0100, Markus Armbruster wrote:
>> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
>> index 6eab7f7..74155fb 100644
>> --- a/hw/9pfs/virtio-9p-device.c
>> +++ b/hw/9pfs/virtio-9p-device.c
>> @@ -94,7 +94,7 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
>>          exit(1);
>>      }
>>  
>> -    s->tag = strdup(conf->tag);
>> +    s->tag = g_strdup(conf->tag);
>>      s->ctx.uid = -1;
>>  
>>      s->ops = fse->ops;
>
> s->tag is leaked.  Want to send a follow-up patch to g_free() it?

I'll give it a try.
Markus Armbruster Jan. 22, 2013, 10:23 a.m. UTC | #3 
Markus Armbruster <armbru@redhat.com> writes:

> Stefan Hajnoczi <stefanha@gmail.com> writes:
>
>> On Wed, Jan 16, 2013 at 06:32:12PM +0100, Markus Armbruster wrote:
>>> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
>>> index 6eab7f7..74155fb 100644
>>> --- a/hw/9pfs/virtio-9p-device.c
>>> +++ b/hw/9pfs/virtio-9p-device.c
>>> @@ -94,7 +94,7 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
>>>          exit(1);
>>>      }
>>>  
>>> -    s->tag = strdup(conf->tag);
>>> +    s->tag = g_strdup(conf->tag);
>>>      s->ctx.uid = -1;
>>>  
>>>      s->ops = fse->ops;
>>
>> s->tag is leaked.  Want to send a follow-up patch to g_free() it?
>
> I'll give it a try.

Mind if I wait for Fred Konrad's virtio refactoring to settle?  It
should make the fix easier, and avoid getting into his way with
pointless conflicts.
fred.konrad@greensocs.com Jan. 22, 2013, 1:51 p.m. UTC | #4 
On 22/01/2013 11:23, Markus Armbruster wrote:
> Markus Armbruster <armbru@redhat.com> writes:
>
>> Stefan Hajnoczi <stefanha@gmail.com> writes:
>>
>>> On Wed, Jan 16, 2013 at 06:32:12PM +0100, Markus Armbruster wrote:
>>>> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
>>>> index 6eab7f7..74155fb 100644
>>>> --- a/hw/9pfs/virtio-9p-device.c
>>>> +++ b/hw/9pfs/virtio-9p-device.c
>>>> @@ -94,7 +94,7 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
>>>>           exit(1);
>>>>       }
>>>>   
>>>> -    s->tag = strdup(conf->tag);
>>>> +    s->tag = g_strdup(conf->tag);
>>>>       s->ctx.uid = -1;
>>>>   
>>>>       s->ops = fse->ops;
>>> s->tag is leaked.  Want to send a follow-up patch to g_free() it?
>> I'll give it a try.
> Mind if I wait for Fred Konrad's virtio refactoring to settle?  It
> should make the fix easier, and avoid getting into his way with
> pointless conflicts.
Hi,

Maybe it's due to missing exit function?

Do you want me to add an exit function to g_free this conf->tag?

Fred
Markus Armbruster Jan. 22, 2013, 3 p.m. UTC | #5 
KONRAD Frédéric <fred.konrad@greensocs.com> writes:

> On 22/01/2013 11:23, Markus Armbruster wrote:
>> Markus Armbruster <armbru@redhat.com> writes:
>>
>>> Stefan Hajnoczi <stefanha@gmail.com> writes:
>>>
>>>> On Wed, Jan 16, 2013 at 06:32:12PM +0100, Markus Armbruster wrote:
>>>>> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
>>>>> index 6eab7f7..74155fb 100644
>>>>> --- a/hw/9pfs/virtio-9p-device.c
>>>>> +++ b/hw/9pfs/virtio-9p-device.c
>>>>> @@ -94,7 +94,7 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
>>>>>           exit(1);
>>>>>       }
>>>>>   -    s->tag = strdup(conf->tag);
>>>>> +    s->tag = g_strdup(conf->tag);
>>>>>       s->ctx.uid = -1;
>>>>>         s->ops = fse->ops;
>>>> s->tag is leaked.  Want to send a follow-up patch to g_free() it?
>>> I'll give it a try.
>> Mind if I wait for Fred Konrad's virtio refactoring to settle?  It
>> should make the fix easier, and avoid getting into his way with
>> pointless conflicts.
> Hi,
>
> Maybe it's due to missing exit function?
>
> Do you want me to add an exit function to g_free this conf->tag?

Yes, please!
diff mbox

Patch

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 6eab7f7..74155fb 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -94,7 +94,7 @@  VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
         exit(1);
     }
 
-    s->tag = strdup(conf->tag);
+    s->tag = g_strdup(conf->tag);
     s->ctx.uid = -1;
 
     s->ops = fse->ops;
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 1136021..f1b1c83 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -46,7 +46,7 @@  static const char *local_mapped_attr_path(FsContext *ctx,
                                           const char *path, char *buffer)
 {
     char *dir_name;
-    char *tmp_path = strdup(path);
+    char *tmp_path = g_strdup(path);
     char *base_name = basename(tmp_path);
 
     /* NULL terminate the directory */
@@ -55,7 +55,7 @@  static const char *local_mapped_attr_path(FsContext *ctx,
 
     snprintf(buffer, PATH_MAX, "%s/%s/%s/%s",
              ctx->fs_root, dir_name, VIRTFS_META_DIR, base_name);
-    free(tmp_path);
+    g_free(tmp_path);
     return buffer;
 }
 
@@ -130,7 +130,7 @@  static int local_create_mapped_attr_dir(FsContext *ctx, const char *path)
 {
     int err;
     char attr_dir[PATH_MAX];
-    char *tmp_path = strdup(path);
+    char *tmp_path = g_strdup(path);
 
     snprintf(attr_dir, PATH_MAX, "%s/%s/%s",
              ctx->fs_root, dirname(tmp_path), VIRTFS_META_DIR);
@@ -139,7 +139,7 @@  static int local_create_mapped_attr_dir(FsContext *ctx, const char *path)
     if (err < 0 && errno == EEXIST) {
         err = 0;
     }
-    free(tmp_path);
+    g_free(tmp_path);
     return err;
 }