[Qemu-devel] [PATCH 0/3] bitsized task for glib conversion

[Qemu-devel] [PATCH 0/3] bitsized task for glib conversion

[Wei Jiangang]

The series used to change basename and dirname to
g_path_get_basename() and g_path_get_dirname() respectively.

Refer to http://wiki.qemu.org/BiteSizedTasks#API_conversion

*** BLURB HERE ***

Wei Jiangang (3):
  linux-user: complete omission of removing uses of strdup
  use g_path_get_dirname instead of dirname
  use g_path_get_basename instead of basename

 fsdev/virtfs-proxy-helper.c |  6 +++++-
 hw/9pfs/9p-local.c          |  6 +++---
 hw/vfio/pci.c               |  6 ++++--
 hw/vfio/platform.c          |  6 ++++--
 linux-user/elfload.c        |  7 ++-----
 os-posix.c                  |  3 ++-
 qemu-io.c                   | 33 ++++++++++++++++++++-------------
 qemu-nbd.c                  |  4 +++-
 qga/commands-posix.c        |  4 ++--
 util/oslib-posix.c          |  4 +++-
 10 files changed, 48 insertions(+), 31 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 1/3] linux-user: complete omission of removing uses of strdup

[Wei Jiangang]

The 900cfbc just removed two unchecked uses of strdup
in fill_psinfo and missed the rest in core_dump_filename.
This patch fixes it.

Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
---
 linux-user/elfload.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index e47caff..6373320 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2718,7 +2718,6 @@ static int core_dump_filename(const TaskState *ts, char *buf,
                               size_t bufsize)
 {
     char timestamp[64];
-    char *filename = NULL;
     char *base_filename = NULL;
     struct timeval tv;
     struct tm tm;
@@ -2731,14 +2730,12 @@ static int core_dump_filename(const TaskState *ts, char *buf,
         return (-1);
     }
 
-    filename = strdup(ts->bprm->filename);
-    base_filename = strdup(basename(filename));
+    base_filename = g_path_get_basename(ts->bprm->filename);
     (void) strftime(timestamp, sizeof (timestamp), "%Y%m%d-%H%M%S",
                     localtime_r(&tv.tv_sec, &tm));
     (void) snprintf(buf, bufsize, "qemu_%s_%s_%d.core",
                     base_filename, timestamp, (int)getpid());
-    free(base_filename);
-    free(filename);
+    g_free(base_filename);
 
     return (0);
 }
-- 
1.9.3

[Qemu-devel] [PATCH 2/3] use g_path_get_dirname instead of dirname

[Wei Jiangang]

Use g_path_get_basename to get the directory components of
a file name, and free its return when no longer needed.

Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
---
 os-posix.c         | 3 ++-
 util/oslib-posix.c | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 107fde3..bcaef17 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -90,7 +90,7 @@ char *os_find_datadir(void)
     if (exec_dir == NULL) {
         return NULL;
     }
-    dir = dirname(exec_dir);
+    dir = g_path_get_dirname(exec_dir);
 
     max_len = strlen(dir) +
         MAX(strlen(SHARE_SUFFIX), strlen(BUILD_SUFFIX)) + 1;
@@ -104,6 +104,7 @@ char *os_find_datadir(void)
         }
     }
 
+    g_free(dir);
     g_free(exec_dir);
     return res;
 }
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 20ca141..1d3248c 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -312,9 +312,11 @@ void qemu_init_exec_dir(const char *argv0)
             return;
         }
     }
-    dir = dirname(p);
+    dir = g_path_get_dirname(p);
 
     pstrcpy(exec_dir, sizeof(exec_dir), dir);
+
+    g_free(dir);
 }
 
 char *qemu_get_exec_dir(void)
-- 
1.9.3

[Qemu-devel] [PATCH 3/3] use g_path_get_basename instead of basename

[Wei Jiangang]

Using g_strdup and g_basename to get the last component
of filename is not the best solution,
Only g_path_get_basename can achive the purpose we want.

Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
---
 fsdev/virtfs-proxy-helper.c |  6 +++++-
 hw/9pfs/9p-local.c          |  6 +++---
 hw/vfio/pci.c               |  6 ++++--
 hw/vfio/platform.c          |  6 ++++--
 qemu-io.c                   | 33 ++++++++++++++++++++-------------
 qemu-nbd.c                  |  4 +++-
 qga/commands-posix.c        |  4 ++--
 7 files changed, 41 insertions(+), 24 deletions(-)

diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index 54f7ad1..a0d6118 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -787,6 +787,8 @@ error:
 
 static void usage(char *prog)
 {
+    char *base_filename = g_path_get_basename(prog);
+
     fprintf(stderr, "usage: %s\n"
             " -p|--path <path> 9p path to export\n"
             " {-f|--fd <socket-descriptor>} socket file descriptor to be used\n"
@@ -795,7 +797,9 @@ static void usage(char *prog)
             " access to this socket\n"
             " \tNote: -s & -f can not be used together\n"
             " [-n|--nodaemon] Run as a normal program\n",
-            basename(prog));
+            base_filename);
+
+    g_free(base_filename);
 }
 
 static int process_reply(int sock, int type,
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 16f45f4..4e6c17a 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -160,17 +160,17 @@ static int local_create_mapped_attr_dir(FsContext *ctx, const char *path)
 {
     int err;
     char *attr_dir;
-    char *tmp_path = g_strdup(path);
+    char *base_filename = g_path_get_basename(path);
 
     attr_dir = g_strdup_printf("%s/%s/%s",
-             ctx->fs_root, dirname(tmp_path), VIRTFS_META_DIR);
+             ctx->fs_root, base_filename, VIRTFS_META_DIR);
 
     err = mkdir(attr_dir, 0700);
     if (err < 0 && errno == EEXIST) {
         err = 0;
     }
     g_free(attr_dir);
-    g_free(tmp_path);
+    g_free(base_filename);
     return err;
 }
 
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index d091d8c..d23b871 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2413,7 +2413,7 @@ static int vfio_initfn(PCIDevice *pdev)
         return -errno;
     }
 
-    vdev->vbasedev.name = g_strdup(basename(vdev->vbasedev.sysfsdev));
+    vdev->vbasedev.name = g_path_get_basename(vdev->vbasedev.sysfsdev);
     vdev->vbasedev.ops = &vfio_pci_ops;
     vdev->vbasedev.type = VFIO_DEVICE_TYPE_PCI;
 
@@ -2428,11 +2428,13 @@ static int vfio_initfn(PCIDevice *pdev)
 
     group_path[len] = 0;
 
-    group_name = basename(group_path);
+    group_name = g_path_get_basename(group_path);
     if (sscanf(group_name, "%d", &groupid) != 1) {
         error_report("vfio: error reading %s: %m", group_path);
+        g_free(group_name);
         return -errno;
     }
+    g_free(group_name);
 
     trace_vfio_initfn(vdev->vbasedev.name, groupid);
 
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 1798a00..47134db 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -557,7 +557,7 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
     /* @sysfsdev takes precedence over @host */
     if (vbasedev->sysfsdev) {
         g_free(vbasedev->name);
-        vbasedev->name = g_strdup(basename(vbasedev->sysfsdev));
+        vbasedev->name = g_path_get_basename(vbasedev->sysfsdev);
     } else {
         if (!vbasedev->name || strchr(vbasedev->name, '/')) {
             return -EINVAL;
@@ -584,11 +584,13 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
 
     group_path[len] = 0;
 
-    group_name = basename(group_path);
+    group_name = g_path_get_basename(group_path);
     if (sscanf(group_name, "%d", &groupid) != 1) {
         error_report("vfio: error reading %s: %m", group_path);
+        g_free(group_name);
         return -errno;
     }
+    g_free(group_name);
 
     trace_vfio_platform_base_device_init(vbasedev->name, groupid);
 
diff --git a/qemu-io.c b/qemu-io.c
index 0a738f1..2f5c616 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -249,6 +249,12 @@ static char *get_prompt(void)
     return prompt;
 }
 
+static void cleanup_and_exit(int status)
+{
+    g_free(progname);
+    exit(status);
+}
+
 static void GCC_FMT_ATTR(2, 3) readline_printf_func(void *opaque,
                                                     const char *fmt, ...)
 {
@@ -440,7 +446,7 @@ int main(int argc, char **argv)
     signal(SIGPIPE, SIG_IGN);
 #endif
 
-    progname = basename(argv[0]);
+    progname = g_path_get_basename(argv[0]);
     qemu_init_exec_dir(argv[0]);
 
     module_call_init(MODULE_INIT_QOM);
@@ -459,7 +465,7 @@ int main(int argc, char **argv)
         case 'd':
             if (bdrv_parse_discard_flags(optarg, &flags) < 0) {
                 error_report("Invalid discard option: %s", optarg);
-                exit(1);
+                cleanup_and_exit(1);
             }
             break;
         case 'f':
@@ -480,26 +486,26 @@ int main(int argc, char **argv)
         case 't':
             if (bdrv_parse_cache_mode(optarg, &flags, &writethrough) < 0) {
                 error_report("Invalid cache option: %s", optarg);
-                exit(1);
+                cleanup_and_exit(1);
             }
             break;
         case 'T':
             if (!trace_init_backends()) {
-                exit(1); /* error message will have been printed */
+                cleanup_and_exit(1); /* error message will have been printed */
             }
             break;
         case 'V':
             printf("%s version %s\n", progname, QEMU_VERSION);
-            exit(0);
+            cleanup_and_exit(0);
         case 'h':
             usage(progname);
-            exit(0);
+            cleanup_and_exit(0);
         case OPTION_OBJECT: {
             QemuOpts *qopts;
             qopts = qemu_opts_parse_noisily(&qemu_object_opts,
                                             optarg, true);
             if (!qopts) {
-                exit(1);
+                cleanup_and_exit(1);
             }
         }   break;
         case OPTION_IMAGE_OPTS:
@@ -507,30 +513,30 @@ int main(int argc, char **argv)
             break;
         default:
             usage(progname);
-            exit(1);
+            cleanup_and_exit(1);
         }
     }
 
     if ((argc - optind) > 1) {
         usage(progname);
-        exit(1);
+        cleanup_and_exit(1);
     }
 
     if (format && imageOpts) {
         error_report("--image-opts and -f are mutually exclusive");
-        exit(1);
+        cleanup_and_exit(1);
     }
 
     if (qemu_init_main_loop(&local_error)) {
         error_report_err(local_error);
-        exit(1);
+        cleanup_and_exit(1);
     }
 
     if (qemu_opts_foreach(&qemu_object_opts,
                           user_creatable_add_opts_foreach,
                           NULL, &local_error)) {
         error_report_err(local_error);
-        exit(1);
+        cleanup_and_exit(1);
     }
 
     /* initialize commands */
@@ -557,7 +563,7 @@ int main(int argc, char **argv)
             QemuOpts *qopts = NULL;
             qopts = qemu_opts_parse_noisily(&file_opts, argv[optind], false);
             if (!qopts) {
-                exit(1);
+                cleanup_and_exit(1);
             }
             opts = qemu_opts_to_qdict(qopts, NULL);
             openfile(NULL, flags, writethrough, opts);
@@ -578,5 +584,6 @@ int main(int argc, char **argv)
 
     blk_unref(qemuio_blk);
     g_free(readline_state);
+    g_free(progname);
     return 0;
 }
diff --git a/qemu-nbd.c b/qemu-nbd.c
index ca4a724..c54d82f 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -808,7 +808,9 @@ int main(int argc, char **argv)
 
     if (device != NULL && sockpath == NULL) {
         sockpath = g_malloc(128);
-        snprintf(sockpath, 128, SOCKET_PATH, basename(device));
+        char *base_filename = g_path_get_basename(device);
+        snprintf(sockpath, 128, SOCKET_PATH, base_filename);
+        g_free(base_filename);
     }
 
     saddr = nbd_build_socket_address(sockpath, bindto, port);
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 2ae3725..cc8736b 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -805,7 +805,7 @@ static char *get_pci_driver(char const *syspath, int pathlen, Error **errp)
     len = readlink(dpath, buf, sizeof(buf) - 1);
     if (len != -1) {
         buf[len] = 0;
-        driver = g_strdup(basename(buf));
+        driver = g_path_get_basename(buf);
     }
     g_free(dpath);
     g_free(path);
@@ -1048,7 +1048,7 @@ static void build_guest_fsinfo_for_device(char const *devpath,
     }
 
     if (!fs->name) {
-        fs->name = g_strdup(basename(syspath));
+        fs->name = g_path_get_basename(syspath);
     }
 
     g_debug("  parse sysfs path '%s'", syspath);
-- 
1.9.3

Re: [Qemu-devel] [PATCH 0/3] bitsized task for glib conversion

[Wei, Jiangang]

ping...
Any comments? Thanks in advance.
On Thu, 2016-04-07 at 10:46 +0800, Wei Jiangang wrote:
> The series used to change basename and dirname to
> g_path_get_basename() and g_path_get_dirname() respectively.
> 
> Refer to http://wiki.qemu.org/BiteSizedTasks#API_conversion
> 
> *** BLURB HERE ***
> 
> Wei Jiangang (3):
>   linux-user: complete omission of removing uses of strdup
>   use g_path_get_dirname instead of dirname
>   use g_path_get_basename instead of basename
> 
>  fsdev/virtfs-proxy-helper.c |  6 +++++-
>  hw/9pfs/9p-local.c          |  6 +++---
>  hw/vfio/pci.c               |  6 ++++--
>  hw/vfio/platform.c          |  6 ++++--
>  linux-user/elfload.c        |  7 ++-----
>  os-posix.c                  |  3 ++-
>  qemu-io.c                   | 33 ++++++++++++++++++++-------------
>  qemu-nbd.c                  |  4 +++-
>  qga/commands-posix.c        |  4 ++--
>  util/oslib-posix.c          |  4 +++-
>  10 files changed, 48 insertions(+), 31 deletions(-)
> 

Re: [Qemu-devel] [PATCH 1/3] linux-user: complete omission of removing uses of strdup

[Paolo Bonzini]

On 07/04/2016 04:46, Wei Jiangang wrote:
> The 900cfbc just removed two unchecked uses of strdup
> in fill_psinfo and missed the rest in core_dump_filename.
> This patch fixes it.
> 
> Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
> ---
>  linux-user/elfload.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index e47caff..6373320 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2718,7 +2718,6 @@ static int core_dump_filename(const TaskState *ts, char *buf,
>                                size_t bufsize)
>  {
>      char timestamp[64];
> -    char *filename = NULL;
>      char *base_filename = NULL;
>      struct timeval tv;
>      struct tm tm;
> @@ -2731,14 +2730,12 @@ static int core_dump_filename(const TaskState *ts, char *buf,
>          return (-1);
>      }
>  
> -    filename = strdup(ts->bprm->filename);
> -    base_filename = strdup(basename(filename));
> +    base_filename = g_path_get_basename(ts->bprm->filename);
>      (void) strftime(timestamp, sizeof (timestamp), "%Y%m%d-%H%M%S",
>                      localtime_r(&tv.tv_sec, &tm));
>      (void) snprintf(buf, bufsize, "qemu_%s_%s_%d.core",
>                      base_filename, timestamp, (int)getpid());
> -    free(base_filename);
> -    free(filename);
> +    g_free(base_filename);
>  
>      return (0);
>  }
> 

This patch fell through the cracks, I think.

Paolo