[Qemu-devel] [PATCH] char: Enable build of pty on macOS

[Qemu-devel] [PATCH] char: Enable build of pty on macOS

[Roman Bolshakov]

For some reason __APPLE__ was not checked in pty code. pty chardev
should be available on macOS, according to man page.

Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 chardev/char-pty.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index 68fd4e20c3..cb00257ebe 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -33,7 +33,7 @@
 
 #if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__)      \
     || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
-    || defined(__GLIBC__)
+    || defined(__GLIBC__) || defined(__APPLE__)
 
 typedef struct {
     Chardev parent;
-- 
2.15.2 (Apple Git-101.1)

Re: [Qemu-devel] [PATCH] char: Enable build of pty on macOS

[Eric Blake]

On 08/21/2018 12:23 PM, Roman Bolshakov wrote:
> For some reason __APPLE__ was not checked in pty code. pty chardev
> should be available on macOS, according to man page.
> 
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
>   chardev/char-pty.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> index 68fd4e20c3..cb00257ebe 100644
> --- a/chardev/char-pty.c
> +++ b/chardev/char-pty.c
> @@ -33,7 +33,7 @@
>   
>   #if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__)      \
>       || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
> -    || defined(__GLIBC__)
> +    || defined(__GLIBC__) || defined(__APPLE__)
> 

Rather than maintaining an ever-growing fragile list of platforms, could 
we instead replace this whole mess with a single define determined at 
configure time based on a feature test?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] char: Enable build of pty on macOS

[Peter Maydell]

On 21 August 2018 at 18:23, Roman Bolshakov <r.bolshakov@yadro.com> wrote:
> For some reason __APPLE__ was not checked in pty code. pty chardev
> should be available on macOS, according to man page.
>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
>  chardev/char-pty.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> index 68fd4e20c3..cb00257ebe 100644
> --- a/chardev/char-pty.c
> +++ b/chardev/char-pty.c
> @@ -33,7 +33,7 @@
>
>  #if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__)      \
>      || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
> -    || defined(__GLIBC__)
> +    || defined(__GLIBC__) || defined(__APPLE__)

We should fix this by figuring out what the code is actually looking
for (ie what OS functions), having a configure test for those
functions, and dropping the big long list of OS ifdefs. Otherwise
we've just got exactly the same problem for the next unix-ish
OS that comes along...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] char: Enable build of pty on macOS

[Paolo Bonzini]

On 21/08/2018 20:29, Peter Maydell wrote:
>>
>>  #if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__)      \
>>      || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
>> -    || defined(__GLIBC__)
>> +    || defined(__GLIBC__) || defined(__APPLE__)
> We should fix this by figuring out what the code is actually looking
> for (ie what OS functions), having a configure test for those
> functions, and dropping the big long list of OS ifdefs. Otherwise
> we've just got exactly the same problem for the next unix-ish
> OS that comes along...

It's really looking only for qemu_openpty_raw, which in turn is compiled
for all CONFIG_POSIX systems.  Because the file is already compiled for
CONFIG_POSIX only, the #ifdef is a legacy of the time before
chardev/char-pty.c was split out of qemu-char.c.  It can be removed.

Paolo