[PATCH] util/oslib-posix : qemu_init_exec_dir implementation for MacOS

[PATCH] util/oslib-posix : qemu_init_exec_dir implementation for MacOS

[David CARLIER]

From b24a6702beb2a4e2a9c1c03b69c6d1dd07d4cf08 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: current process full path resolution on MacOS

Using existing libproc to fill the path.

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

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 062236a1ab..96f0405ee6 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -55,6 +55,10 @@
 #include <sys/sysctl.h>
 #endif

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

 #ifdef CONFIG_DEBUG_STACK_USAGE
@@ -366,6 +370,15 @@ void qemu_init_exec_dir(const char *argv0)
             p = buf;
         }
     }
+#elif defined(__APPLE__)
+    {
+        uint32_t len;
+        len = proc_pidpath(getpid(), buf, sizeof(buf) - 1);
+        if (len > 0) {
+            buf[len] = 0;
+            p = buf;
+        }
+    }
 #endif
     /* If we don't have any way of figuring out the actual executable
        location then try argv[0].  */
-- 
2.26.2

Re: [PATCH] util/oslib-posix : qemu_init_exec_dir implementation for MacOS

[David CARLIER]

From ce857629697e8b6a2149fd3a1e16b7eea26aafca 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: current process full path resolution on MacOS

Using existing libproc to fill the path.

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

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 062236a1ab..445af2f9be 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -55,6 +55,10 @@
 #include <sys/sysctl.h>
 #endif

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

 #ifdef CONFIG_DEBUG_STACK_USAGE
@@ -366,6 +370,15 @@ void qemu_init_exec_dir(const char *argv0)
             p = buf;
         }
     }
+#elif defined(__APPLE__)
+    {
+        int len;
+        len = proc_pidpath(getpid(), buf, sizeof(buf) - 1);
+        if (len > 0) {
+            buf[len] = 0;
+            p = buf;
+        }
+    }
 #endif
     /* If we don't have any way of figuring out the actual executable
        location then try argv[0].  */
-- 
2.26.2

On Tue, 26 May 2020 at 21:40, David CARLIER <devnexen@gmail.com> wrote:
>
> From b24a6702beb2a4e2a9c1c03b69c6d1dd07d4cf08 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: current process full path resolution on MacOS
>
> Using existing libproc to fill the path.
>
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
>  util/oslib-posix.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 062236a1ab..96f0405ee6 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -55,6 +55,10 @@
>  #include <sys/sysctl.h>
>  #endif
>
> +#ifdef __APPLE__
> +#include <libproc.h>
> +#endif
> +
>  #include "qemu/mmap-alloc.h"
>
>  #ifdef CONFIG_DEBUG_STACK_USAGE
> @@ -366,6 +370,15 @@ void qemu_init_exec_dir(const char *argv0)
>              p = buf;
>          }
>      }
> +#elif defined(__APPLE__)
> +    {
> +        uint32_t len;
> +        len = proc_pidpath(getpid(), buf, sizeof(buf) - 1);
> +        if (len > 0) {
> +            buf[len] = 0;
> +            p = buf;
> +        }
> +    }
>  #endif
>      /* If we don't have any way of figuring out the actual executable
>         location then try argv[0].  */
> --
> 2.26.2

Re: [PATCH] util/oslib-posix : qemu_init_exec_dir implementation for MacOS

[Philippe Mathieu-Daudé]

Cc'ing more developers.

On 5/26/20 10:40 PM, David CARLIER wrote:
> From b24a6702beb2a4e2a9c1c03b69c6d1dd07d4cf08 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: current process full path resolution on MacOS
> 
> Using existing libproc to fill the path.
> 
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
>  util/oslib-posix.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 062236a1ab..96f0405ee6 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -55,6 +55,10 @@
>  #include <sys/sysctl.h>
>  #endif
> 
> +#ifdef __APPLE__
> +#include <libproc.h>
> +#endif
> +
>  #include "qemu/mmap-alloc.h"
> 
>  #ifdef CONFIG_DEBUG_STACK_USAGE
> @@ -366,6 +370,15 @@ void qemu_init_exec_dir(const char *argv0)
>              p = buf;
>          }
>      }
> +#elif defined(__APPLE__)
> +    {
> +        uint32_t len;
> +        len = proc_pidpath(getpid(), buf, sizeof(buf) - 1);
> +        if (len > 0) {
> +            buf[len] = 0;
> +            p = buf;
> +        }
> +    }
>  #endif
>      /* If we don't have any way of figuring out the actual executable
>         location then try argv[0].  */
> 

Re: [PATCH] util/oslib-posix : qemu_init_exec_dir implementation for MacOS

[David CARLIER]

Little bit better the second version of the patch, difficult to sort
out things with mailing list :-)
From ce857629697e8b6a2149fd3a1e16b7eea26aafca 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: current process full path resolution on MacOS

Using existing libproc to fill the path.

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

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 062236a1ab..445af2f9be 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -55,6 +55,10 @@
 #include <sys/sysctl.h>
 #endif

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

 #ifdef CONFIG_DEBUG_STACK_USAGE
@@ -366,6 +370,15 @@ void qemu_init_exec_dir(const char *argv0)
             p = buf;
         }
     }
+#elif defined(__APPLE__)
+    {
+        int len;

+        len = proc_pidpath(getpid(), buf, sizeof(buf) - 1);
+        if (len > 0) {
+            buf[len] = 0;
+            p = buf;
+        }
+    }
 #endif
     /* If we don't have any way of figuring out the actual executable
        location then try argv[0].  */
--
2.26.2

On Wed, 3 Jun 2020 at 07:08, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Cc'ing more developers.
>
> On 5/26/20 10:40 PM, David CARLIER wrote:
> > From b24a6702beb2a4e2a9c1c03b69c6d1dd07d4cf08 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: current process full path resolution on MacOS
> >
> > Using existing libproc to fill the path.
> >
> > Signed-off-by: David Carlier <devnexen@gmail.com>
> > ---
> >  util/oslib-posix.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> > index 062236a1ab..96f0405ee6 100644
> > --- a/util/oslib-posix.c
> > +++ b/util/oslib-posix.c
> > @@ -55,6 +55,10 @@
> >  #include <sys/sysctl.h>
> >  #endif
> >
> > +#ifdef __APPLE__
> > +#include <libproc.h>
> > +#endif
> > +
> >  #include "qemu/mmap-alloc.h"
> >
> >  #ifdef CONFIG_DEBUG_STACK_USAGE
> > @@ -366,6 +370,15 @@ void qemu_init_exec_dir(const char *argv0)
> >              p = buf;
> >          }
> >      }
> > +#elif defined(__APPLE__)
> > +    {
> > +        uint32_t len;
> > +        len = proc_pidpath(getpid(), buf, sizeof(buf) - 1);
> > +        if (len > 0) {
> > +            buf[len] = 0;
> > +            p = buf;
> > +        }
> > +    }
> >  #endif
> >      /* If we don't have any way of figuring out the actual executable
> >         location then try argv[0].  */
> >
>

Re: [PATCH] util/oslib-posix : qemu_init_exec_dir implementation for MacOS

[Justin Hibbits]

On Wed, 3 Jun 2020 08:08:42 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Cc'ing more developers.
> 
> On 5/26/20 10:40 PM, David CARLIER wrote:
> > From b24a6702beb2a4e2a9c1c03b69c6d1dd07d4cf08 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: current process full path resolution
> > on MacOS
> > 
> > Using existing libproc to fill the path.
> > 
> > Signed-off-by: David Carlier <devnexen@gmail.com>
> > ---
> >  util/oslib-posix.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> > index 062236a1ab..96f0405ee6 100644
> > --- a/util/oslib-posix.c
> > +++ b/util/oslib-posix.c
> > @@ -55,6 +55,10 @@
> >  #include <sys/sysctl.h>
> >  #endif
> > 
> > +#ifdef __APPLE__
> > +#include <libproc.h>
> > +#endif
> > +
> >  #include "qemu/mmap-alloc.h"
> > 
> >  #ifdef CONFIG_DEBUG_STACK_USAGE
> > @@ -366,6 +370,15 @@ void qemu_init_exec_dir(const char *argv0)
> >              p = buf;
> >          }
> >      }
> > +#elif defined(__APPLE__)
> > +    {
> > +        uint32_t len;
> > +        len = proc_pidpath(getpid(), buf, sizeof(buf) - 1);
> > +        if (len > 0) {
> > +            buf[len] = 0;
> > +            p = buf;
> > +        }
> > +    }
> >  #endif
> >      /* If we don't have any way of figuring out the actual
> > executable location then try argv[0].  */
> >   
> 

Apologies, I don't have context for this.  Why was I CC'd on this?

Does proc_pidpath() not NUL-terminate its written string?  And, given
from my quick google search, it looks like this function is private and
subject to change, so can you guarantee that the returned length is the
*written* length, not the full string length?  If not, you could be
overwriting other arbitrary data.

- Justin

Re: [PATCH] util/oslib-posix : qemu_init_exec_dir implementation for MacOS

[David CARLIER]

Good point even tough the libproc api is here in this form since quite a time.

From d23bf60961ee036f8298794f879d1b8b9bd717dc 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: current process full path resolution on MacOS

Using existing libproc to fill the path.

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

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 062236a1ab..9dd1e1a18b 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -55,6 +55,10 @@
 #include <sys/sysctl.h>
 #endif

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

 #ifdef CONFIG_DEBUG_STACK_USAGE
@@ -366,6 +370,15 @@ void qemu_init_exec_dir(const char *argv0)
             p = buf;
         }
     }
+#elif defined(__APPLE__)
+    {
+        int len;
+        len = proc_pidpath(getpid(), buf, sizeof(buf) - 1);
+        if (len <= 0) {
+            return;
+        }
+        p = buf;
+    }
 #endif
     /* If we don't have any way of figuring out the actual executable
        location then try argv[0].  */
-- 
2.26.2

On Wed, 3 Jun 2020 at 15:09, Justin Hibbits <chmeeedalf@gmail.com> wrote:
>
> On Wed, 3 Jun 2020 08:08:42 +0200
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> > Cc'ing more developers.
> >
> > On 5/26/20 10:40 PM, David CARLIER wrote:
> > > From b24a6702beb2a4e2a9c1c03b69c6d1dd07d4cf08 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: current process full path resolution
> > > on MacOS
> > >
> > > Using existing libproc to fill the path.
> > >
> > > Signed-off-by: David Carlier <devnexen@gmail.com>
> > > ---
> > >  util/oslib-posix.c | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > >
> > > diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> > > index 062236a1ab..96f0405ee6 100644
> > > --- a/util/oslib-posix.c
> > > +++ b/util/oslib-posix.c
> > > @@ -55,6 +55,10 @@
> > >  #include <sys/sysctl.h>
> > >  #endif
> > >
> > > +#ifdef __APPLE__
> > > +#include <libproc.h>
> > > +#endif
> > > +
> > >  #include "qemu/mmap-alloc.h"
> > >
> > >  #ifdef CONFIG_DEBUG_STACK_USAGE
> > > @@ -366,6 +370,15 @@ void qemu_init_exec_dir(const char *argv0)
> > >              p = buf;
> > >          }
> > >      }
> > > +#elif defined(__APPLE__)
> > > +    {
> > > +        uint32_t len;
> > > +        len = proc_pidpath(getpid(), buf, sizeof(buf) - 1);
> > > +        if (len > 0) {
> > > +            buf[len] = 0;
> > > +            p = buf;
> > > +        }
> > > +    }
> > >  #endif
> > >      /* If we don't have any way of figuring out the actual
> > > executable location then try argv[0].  */
> > >
> >
>
> Apologies, I don't have context for this.  Why was I CC'd on this?
>
> Does proc_pidpath() not NUL-terminate its written string?  And, given
> from my quick google search, it looks like this function is private and
> subject to change, so can you guarantee that the returned length is the
> *written* length, not the full string length?  If not, you could be
> overwriting other arbitrary data.
>
> - Justin

Re: [PATCH] util/oslib-posix : qemu_init_exec_dir implementation for MacOS

[Philippe Mathieu-Daudé]

On 6/3/20 4:22 PM, David CARLIER wrote:
> Good point even tough the libproc api is here in this form since quite a time.

Top-posting is difficult to read on technical lists; it's better to
reply inline.

Also, please don't remove the post content you are replying to...
Because then your answer doesn't make much sense out of context.

> 
> From d23bf60961ee036f8298794f879d1b8b9bd717dc 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: current process full path resolution on MacOS
> 
> Using existing libproc to fill the path.
> 
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
>  util/oslib-posix.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 062236a1ab..9dd1e1a18b 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -55,6 +55,10 @@
>  #include <sys/sysctl.h>
>  #endif
> 
> +#ifdef __APPLE__
> +#include <libproc.h>
> +#endif
> +
>  #include "qemu/mmap-alloc.h"
> 
>  #ifdef CONFIG_DEBUG_STACK_USAGE
> @@ -366,6 +370,15 @@ void qemu_init_exec_dir(const char *argv0)
>              p = buf;
>          }
>      }
> +#elif defined(__APPLE__)
> +    {
> +        int len;
> +        len = proc_pidpath(getpid(), buf, sizeof(buf) - 1);
> +        if (len <= 0) {
> +            return;
> +        }
> +        p = buf;
> +    }
>  #endif
>      /* If we don't have any way of figuring out the actual executable
>         location then try argv[0].  */
> 

Re: [PATCH] util/oslib-posix : qemu_init_exec_dir implementation for MacOS

[Peter Maydell]

On Wed, 3 Jun 2020 at 15:23, David CARLIER <devnexen@gmail.com> wrote:
>
> Good point even tough the libproc api is here in this form since quite a time.
>
> From d23bf60961ee036f8298794f879d1b8b9bd717dc 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: current process full path resolution on MacOS
>
> Using existing libproc to fill the path.

Could you send new versions of the patch as their own emails
(ie not replies to the first one) with "[PATCH v2]" "[PATCH v3]"
etc in the subject line, please? Otherwise it gets tricky to
figure out which is the most recent version of the patch.

> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
>  util/oslib-posix.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 062236a1ab..9dd1e1a18b 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -55,6 +55,10 @@
>  #include <sys/sysctl.h>
>  #endif
>
> +#ifdef __APPLE__
> +#include <libproc.h>
> +#endif
> +
>  #include "qemu/mmap-alloc.h"
>
>  #ifdef CONFIG_DEBUG_STACK_USAGE
> @@ -366,6 +370,15 @@ void qemu_init_exec_dir(const char *argv0)
>              p = buf;
>          }
>      }
> +#elif defined(__APPLE__)
> +    {
> +        int len;
> +        len = proc_pidpath(getpid(), buf, sizeof(buf) - 1);
> +        if (len <= 0) {
> +            return;

I think that if proc_pidpath() doesn't work we should fall back
to the "try argv[0]" code below, not return without initializing
exec_dir[]. This is what the existing Linux and BSD codepaths do.

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

I did a bit of searching to see whether there were any alternatives
to proc_pidpath(), and I found _NSGetExecutablePath(). This
function *is* documented:
 https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/dyld.3.html

Will it do what we want here? You'll need to call
realpath() on the results, and we should test that it
actually does better than just looking at argv[0] -- ie
that if you start QEMU via execve(/path/to/qemu,
argv_with_bogus_argv0, ...) then we get the /path/to/qemu,
not whatever the bogus argv0 value was.

There's also the "get the ui/cocoa.m code to find the path
via the AppKit API" approach that Roman suggested, though I
think that is more awkward than _NSGetExecutablePath() as
you'd need to get the ui/cocoa.m code to save the path for you.

If that doesn't work then I guess we can use proc_pidpath(),
but I'd rather avoid that if we can. If we do have to go that
route we should have a comment noting that it's an undocumented
and unsupported API but that it's also used by well-known
applications X, Y...

thanks
-- PMM

Re: [PATCH] util/oslib-posix : qemu_init_exec_dir implementation for MacOS

[Philippe Mathieu-Daudé]

On 6/3/20 4:09 PM, Justin Hibbits wrote:
> On Wed, 3 Jun 2020 08:08:42 +0200
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
>> Cc'ing more developers.
>>
>> On 5/26/20 10:40 PM, David CARLIER wrote:
>>> From b24a6702beb2a4e2a9c1c03b69c6d1dd07d4cf08 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: current process full path resolution
>>> on MacOS
>>>
>>> Using existing libproc to fill the path.
>>>
>>> Signed-off-by: David Carlier <devnexen@gmail.com>
>>> ---
>>>  util/oslib-posix.c | 13 +++++++++++++
>>>  1 file changed, 13 insertions(+)
>>>
>>> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>>> index 062236a1ab..96f0405ee6 100644
>>> --- a/util/oslib-posix.c
>>> +++ b/util/oslib-posix.c
>>> @@ -55,6 +55,10 @@
>>>  #include <sys/sysctl.h>
>>>  #endif
>>>
>>> +#ifdef __APPLE__
>>> +#include <libproc.h>
>>> +#endif
>>> +
>>>  #include "qemu/mmap-alloc.h"
>>>
>>>  #ifdef CONFIG_DEBUG_STACK_USAGE
>>> @@ -366,6 +370,15 @@ void qemu_init_exec_dir(const char *argv0)
>>>              p = buf;
>>>          }
>>>      }
>>> +#elif defined(__APPLE__)
>>> +    {
>>> +        uint32_t len;
>>> +        len = proc_pidpath(getpid(), buf, sizeof(buf) - 1);
>>> +        if (len > 0) {
>>> +            buf[len] = 0;
>>> +            p = buf;
>>> +        }
>>> +    }
>>>  #endif
>>>      /* If we don't have any way of figuring out the actual
>>> executable location then try argv[0].  */
>>>   
>>
> 
> Apologies, I don't have context for this.  Why was I CC'd on this?

I did after finding this patch of yours:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg639033.html

> 
> Does proc_pidpath() not NUL-terminate its written string?  And, given
> from my quick google search, it looks like this function is private and
> subject to change, so can you guarantee that the returned length is the
> *written* length, not the full string length?  If not, you could be
> overwriting other arbitrary data.
> 
> - Justin
> 

Re: [PATCH] util/oslib-posix : qemu_init_exec_dir implementation for MacOS

[Justin Hibbits]

On Wed, 3 Jun 2020 16:36:27 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> On 6/3/20 4:09 PM, Justin Hibbits wrote:
> > On Wed, 3 Jun 2020 08:08:42 +0200
> > Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >   
> >> Cc'ing more developers.
> >>
> >> On 5/26/20 10:40 PM, David CARLIER wrote:  
> >>> From b24a6702beb2a4e2a9c1c03b69c6d1dd07d4cf08 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: current process full path resolution
> >>> on MacOS
> >>>
> >>> Using existing libproc to fill the path.
> >>>
> >>> Signed-off-by: David Carlier <devnexen@gmail.com>
> >>> ---
> >>>  util/oslib-posix.c | 13 +++++++++++++
> >>>  1 file changed, 13 insertions(+)
> >>>
> >>> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> >>> index 062236a1ab..96f0405ee6 100644
> >>> --- a/util/oslib-posix.c
> >>> +++ b/util/oslib-posix.c
> >>> @@ -55,6 +55,10 @@
> >>>  #include <sys/sysctl.h>
> >>>  #endif
> >>>
> >>> +#ifdef __APPLE__
> >>> +#include <libproc.h>
> >>> +#endif
> >>> +
> >>>  #include "qemu/mmap-alloc.h"
> >>>
> >>>  #ifdef CONFIG_DEBUG_STACK_USAGE
> >>> @@ -366,6 +370,15 @@ void qemu_init_exec_dir(const char *argv0)
> >>>              p = buf;
> >>>          }
> >>>      }
> >>> +#elif defined(__APPLE__)
> >>> +    {
> >>> +        uint32_t len;
> >>> +        len = proc_pidpath(getpid(), buf, sizeof(buf) - 1);
> >>> +        if (len > 0) {
> >>> +            buf[len] = 0;
> >>> +            p = buf;
> >>> +        }
> >>> +    }
> >>>  #endif
> >>>      /* If we don't have any way of figuring out the actual
> >>> executable location then try argv[0].  */
> >>>     
> >>  
> > 
> > Apologies, I don't have context for this.  Why was I CC'd on this?  
> 
> I did after finding this patch of yours:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg639033.html

Ah, okay, thanks.  I haven't contributed much to qemu, so was curious.

- Justin

Re: [PATCH] util/oslib-posix : qemu_init_exec_dir implementation for MacOS

[Roman Bolshakov]

On Tue, May 26, 2020 at 09:40:27PM +0100, David CARLIER wrote:
> From b24a6702beb2a4e2a9c1c03b69c6d1dd07d4cf08 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: current process full path resolution on MacOS
> 
> Using existing libproc to fill the path.
> 
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
>  util/oslib-posix.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 062236a1ab..96f0405ee6 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -55,6 +55,10 @@
>  #include <sys/sysctl.h>
>  #endif
> 
> +#ifdef __APPLE__
> +#include <libproc.h>
> +#endif
> +
>  #include "qemu/mmap-alloc.h"
> 
>  #ifdef CONFIG_DEBUG_STACK_USAGE
> @@ -366,6 +370,15 @@ void qemu_init_exec_dir(const char *argv0)
>              p = buf;
>          }
>      }
> +#elif defined(__APPLE__)
> +    {
> +        uint32_t len;
> +        len = proc_pidpath(getpid(), buf, sizeof(buf) - 1);

Hi David,

proc_pidpath handler in the xnu kernel is indirectly calling
build_path_with_parent [1] and it includes NULL terminator into
buffersize, i.e. the patch seems to limit path length to one less
character than OS allows.

> +        if (len > 0) {
> +            buf[len] = 0;
> +            p = buf;
> +        }
> +    }
>  #endif
>      /* If we don't have any way of figuring out the actual executable
>         location then try argv[0].  */
> -- 
> 2.26.2
> 


The change looks okay but it's not clear why it is needed [2].

Also, libproc.h [3] has this comment:

/*
 * This header file contains private interfaces to obtain process information.  
 * These interfaces are subject to change in future releases.
 */

But iTerm2 [4] an Chromium [5] are using it. An alternative (if it works
and IMO less appealing) would be to retrieve process path using AppKit [6].

1. https://opensource.apple.com/source/xnu/xnu-6153.81.5/bsd/vfs/vfs_cache.c.auto.html
2. https://wiki.qemu.org/Contribute/SubmitAPatch#Write_a_meaningful_commit_message
3. https://opensource.apple.com/source/xnu/xnu-6153.81.5/libsyscall/wrappers/libproc/libproc.h.auto.html
4. https://gitlab.com/gnachman/iterm2/blob/872e3d63fbcaf7b4235c4f3b7273e09a7ba4c1d1/sources/iTermLSOF.m#L31
5. https://github.com/chromium/chromium/blob/2ca8c5037021c9d2ecc00b787d58a31ed8fc8bcb/base/process/process_handle_mac.cc#L31
6. https://developer.apple.com/documentation/appkit/nsworkspace?language=objc

Regards,
Roman