diff mbox series

[V4] util/oslib-posix : qemu_init_exec_dir implementation for Mac

Message ID CA+XhMqx6VjRhT6xBzJ-UYs7cPDXVK=PNdfNVdad3Tqhe43P=Ew@mail.gmail.com
State New
Headers show
Series [V4] util/oslib-posix : qemu_init_exec_dir implementation for Mac | expand

Commit Message

David CARLIER June 16, 2020, 4:10 p.m. UTC 
From 7eef8b803cdb0e7148fdf894d2992052695c1ff8 Mon Sep 17 00:00:00 2001
From: David Carlier <devnexen@gmail.com>
Date: Tue, 26 May 2020 21:35:27 +0100
Subject: [PATCH] util/oslib-posix : qemu_init_exec_dir implementation for Mac

Using dyld API to get the full path of the current process.

Signed-off-by: David Carlier <devnexen@gmail.com>
---
 util/oslib-posix.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Peter Maydell June 16, 2020, 6:32 p.m. UTC | #1 
On Tue, 16 Jun 2020 at 17:12, David CARLIER <devnexen@gmail.com> wrote:
>
> From 7eef8b803cdb0e7148fdf894d2992052695c1ff8 Mon Sep 17 00:00:00 2001
> From: David Carlier <devnexen@gmail.com>
> Date: Tue, 26 May 2020 21:35:27 +0100
> Subject: [PATCH] util/oslib-posix : qemu_init_exec_dir implementation for Mac
>
> Using dyld API to get the full path of the current process.
>
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
>  util/oslib-posix.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 916f1be224..3612c2c80e 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -57,6 +57,10 @@
>  #include <lwp.h>
>  #endif
>
> +#ifdef __APPLE__
> +#include <mach-o/dyld.h>
> +#endif
> +
>  #include "qemu/mmap-alloc.h"
>
>  #ifdef CONFIG_DEBUG_STACK_USAGE
> @@ -375,6 +379,16 @@ void qemu_init_exec_dir(const char *argv0)
>              p = buf;
>          }
>      }
> +#elif defined(__APPLE__)
> +    {
> +        uint32_t len = sizeof(buf);
> +        if (_NSGetExecutablePath(buf, &len) == 0) {
> +     char fpath[PATH_MAX];

(Something seems to have gone wrong with the indentation here;
QEMU indent is always-spaces, 4-spaces-per-indent.)

> +            buf[len - 1] = 0;

_NSGetExecutablePath() will 0-terminate the string, so
you don't need to write this 0 here. (The function call
will fail if the buffer is not long enough for both the
path string and the terminating NUL character.)

> +      realpath(buf, fpath);

You need to check the return value from realpath() in
case it returns an error; compare the code just below
that handles calling realpath() on argv0.

> +            p = fpath;

This is setting p to a pointer to a buffer which goes
out of scope, and then using it after it's out of scope,
which is undefined behaviour. You can avoid that by
doing _NSGetExecutablePath() into the tightly-scoped
'fpath[]', and then doing realpath() into 'buf[]'
(which is still live to the end of the function).

> +        }
> +    }
>  #endif
>      /* If we don't have any way of figuring out the actual executable
>         location then try argv[0].  */

thanks
-- PMM
no-reply@patchew.org June 16, 2020, 6:41 p.m. UTC | #2 
Patchew URL: https://patchew.org/QEMU/CA+XhMqx6VjRhT6xBzJ-UYs7cPDXVK=PNdfNVdad3Tqhe43P=Ew@mail.gmail.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH V4] util/oslib-posix : qemu_init_exec_dir implementation for Mac
Type: series
Message-id: CA+XhMqx6VjRhT6xBzJ-UYs7cPDXVK=PNdfNVdad3Tqhe43P=Ew@mail.gmail.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
4ae8a07 util/oslib-posix : qemu_init_exec_dir implementation for Mac

=== OUTPUT BEGIN ===
WARNING: architecture specific defines should be avoided
#25: FILE: util/oslib-posix.c:60:
+#ifdef __APPLE__

ERROR: suspect code indent for conditional statements (8, 5)
#39: FILE: util/oslib-posix.c:385:
+        if (_NSGetExecutablePath(buf, &len) == 0) {
+     char fpath[PATH_MAX];

total: 1 errors, 1 warnings, 26 lines checked

Commit 4ae8a07edc80 (util/oslib-posix : qemu_init_exec_dir implementation for Mac) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/CA+XhMqx6VjRhT6xBzJ-UYs7cPDXVK=PNdfNVdad3Tqhe43P=Ew@mail.gmail.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
diff mbox series

Patch

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 916f1be224..3612c2c80e 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -57,6 +57,10 @@ 
 #include <lwp.h>
 #endif

+#ifdef __APPLE__
+#include <mach-o/dyld.h>
+#endif
+
 #include "qemu/mmap-alloc.h"

 #ifdef CONFIG_DEBUG_STACK_USAGE
@@ -375,6 +379,16 @@  void qemu_init_exec_dir(const char *argv0)
             p = buf;
         }
     }
+#elif defined(__APPLE__)
+    {
+        uint32_t len = sizeof(buf);
+        if (_NSGetExecutablePath(buf, &len) == 0) {
+     char fpath[PATH_MAX];
+            buf[len - 1] = 0;
+      realpath(buf, fpath);
+            p = fpath;
+        }
+    }
 #endif
     /* If we don't have any way of figuring out the actual executable
        location then try argv[0].  */