| Message ID | 20180821172324.29331-1-r.bolshakov@yadro.com |
|---|---|
| State | New |
| Headers | show |
| Series | char: Enable build of pty on macOS | expand |
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?
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
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
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;
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(-)