Patchwork [for-1.5] portability: pty.h is glibc-specific

login
register
mail settings
Submitter Paolo Bonzini
Date May 14, 2013, 12:05 p.m.
Message ID <1368533121-30796-1-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/243691/
State New
Headers show

Comments

Paolo Bonzini - May 14, 2013, 12:05 p.m.
This should fix building the GTK+ front-end on BSDs.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu-common.h | 12 ++++++++++++
 qemu-char.c           | 10 ----------
 ui/gtk.c              |  1 -
 3 files changed, 12 insertions(+), 11 deletions(-)
Anthony Liguori - May 14, 2013, 4:10 p.m.
Applied.  Thanks.

Regards,

Anthony Liguori
Brad - May 14, 2013, 4:15 p.m.
On Tue, May 14, 2013 at 04:10:56PM -0000, Anthony Liguori wrote:
> Applied.  Thanks.
> 
> Regards,
> 
> Anthony Liguori

The patch was not updated to take into consideration my comment from bug 1177774 so
what was commited is just as broken as before the patch.
Anthony Liguori - May 14, 2013, 4:28 p.m.
Brad Smith <brad@comstyle.com> writes:

> On Tue, May 14, 2013 at 04:10:56PM -0000, Anthony Liguori wrote:
>> Applied.  Thanks.
>> 
>> Regards,
>> 
>> Anthony Liguori
>
> The patch was not updated to take into consideration my comment from bug 1177774 so
> what was commited is just as broken as before the patch.

If a patch doesn't solve a problem, respond on the mailing list to the
patch itself.

Regards,

Anthony Liguori

>
> -- 
> This message has been scanned for viruses and
> dangerous content by MailScanner, and is
> believed to be clean.
Paolo Bonzini - May 14, 2013, 4:35 p.m.
Il 14/05/2013 18:15, Brad Smith ha scritto:
> On Tue, May 14, 2013 at 04:10:56PM -0000, Anthony Liguori wrote:
>> Applied.  Thanks.
>>
>> Regards,
>>
>> Anthony Liguori
> 
> The patch was not updated to take into consideration my comment from bug 1177774 so
> what was commited is just as broken as before the patch.

Just send a followup yourself.  You and Blue Swirl are the only users of
QEMU on OpenBSD as far as we know, so anything that you do not do
yourself is just best-effort.

Paolo
Laszlo Ersek - May 14, 2013, 7:28 p.m.
On 05/14/13 18:35, Paolo Bonzini wrote:
> Il 14/05/2013 18:15, Brad Smith ha scritto:
>> On Tue, May 14, 2013 at 04:10:56PM -0000, Anthony Liguori wrote:
>>> Applied.  Thanks.
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>
>> The patch was not updated to take into consideration my comment from bug 1177774 so
>> what was commited is just as broken as before the patch.
> 
> Just send a followup yourself.  You and Blue Swirl are the only users of
> QEMU on OpenBSD as far as we know, so anything that you do not do
> yourself is just best-effort.

Not to argue with this, just a general note: most of whatever is
portable from the terminal IO interfaces belongs into <termios.h> indeed.

http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/termios.h.html

Traditionally nonportable stuff related to pty allocation and
master-slave pairing has been "recently" standardized as well (as in,
SUSv3 / UNIX 03):

http://pubs.opengroup.org/onlinepubs/9699919799/functions/posix_openpt.html

(+grantpt, ptsname, unlockpt; they seem to date back to SUSv1 / UNIX 95)

CC'ing Eric for the obligatory POSIX sanity check... :)

Laszlo
Peter Maydell - May 14, 2013, 7:34 p.m.
On 14 May 2013 13:05, Paolo Bonzini <pbonzini@redhat.com> wrote:
> This should fix building the GTK+ front-end on BSDs.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Hi. This patch breaks the build on MacOSX:

manooth$ make block/vdi.o
	CHK version_gen.h
  CC    block/vdi.o
block/vdi.c:124:20: error: static declaration of 'uuid_generate'
follows non-static declaration
static inline void uuid_generate(uuid_t out)
                   ^
/usr/include/uuid/uuid.h:63:6: note: previous declaration is here
void uuid_generate(uuid_t out);
     ^
block/vdi.c:129:19: error: static declaration of 'uuid_is_null'
follows non-static declaration
static inline int uuid_is_null(const uuid_t uu)
                  ^
/usr/include/uuid/uuid.h:67:5: note: previous declaration is here
int uuid_is_null(const uuid_t uu);
    ^
block/vdi.c:135:20: error: static declaration of 'uuid_unparse'
follows non-static declaration
static inline void uuid_unparse(const uuid_t uu, char *out)
                   ^
/usr/include/uuid/uuid.h:71:6: note: previous declaration is here
void uuid_unparse(const uuid_t uu, uuid_string_t out);
     ^
3 errors generated.
make: *** [block/vdi.o] Error 1


> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -42,6 +42,18 @@
>  #include <signal.h>
>  #include "glib-compat.h"
>
> +#if defined(__GLIBC__)
> +# include <pty.h>
> +#elif defined CONFIG_BSD
> +# if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__DragonFly__)
> +#  include <libutil.h>
> +# else
> +#  include <util.h>
> +# endif
> +#elif defined CONFIG_SOLARIS
> +# include <stropts.h>
> +#endif

On MacOSX, util.h includes pwd.h which includes uuid/uuid.h (all
system headers). The system uuid.h defines then conflict with
QEMU's.

(configure's test for libuuid has failed, so
CONFIG_UUID isn't defined. This is because on MacOS you don't
need '-luuid' and including it on the command line makes the
compile-test fail because of a nonexistent library.)

thanks
-- PMM
Eric Blake - May 14, 2013, 7:59 p.m.
On 05/14/2013 01:28 PM, Laszlo Ersek wrote:

> Not to argue with this, just a general note: most of whatever is
> portable from the terminal IO interfaces belongs into <termios.h> indeed.
> 
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/termios.h.html
> 
> Traditionally nonportable stuff related to pty allocation and
> master-slave pairing has been "recently" standardized as well (as in,
> SUSv3 / UNIX 03):
> 
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/posix_openpt.html
> 
> (+grantpt, ptsname, unlockpt; they seem to date back to SUSv1 / UNIX 95)
> 
> CC'ing Eric for the obligatory POSIX sanity check... :)

Looks accurate.

Still missing from POSIX is ptsname_r, although it might appear in the
next revision ("Issue 8") in a few years:
http://austingroupbugs.net/view.php?id=508

which means right now, POSIX has no thread-safe mechanism for
determining the slave name after opening a master pty.

Personally, I find the high-level BSD openpty() interface a lot nicer to
use than the POSIX posix_openpt()/grantpt()/unlockpt()/ptsname()
sequence, but it may come with its own pitfalls since it is not
standardized (yet).

Patch

diff --git a/include/qemu-common.h b/include/qemu-common.h
index b399d85..af769f5 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -42,6 +42,18 @@ 
 #include <signal.h>
 #include "glib-compat.h"
 
+#if defined(__GLIBC__)
+# include <pty.h>
+#elif defined CONFIG_BSD
+# if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__DragonFly__)
+#  include <libutil.h>
+# else
+#  include <util.h>
+# endif
+#elif defined CONFIG_SOLARIS
+# include <stropts.h>
+#endif
+
 #ifdef _WIN32
 #include "sysemu/os-win32.h"
 #endif
diff --git a/qemu-char.c b/qemu-char.c
index 64e824d..30a2ddf 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -53,13 +53,6 @@ 
 #include <sys/select.h>
 #ifdef CONFIG_BSD
 #include <sys/stat.h>
-#if defined(__GLIBC__)
-#include <pty.h>
-#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__DragonFly__)
-#include <libutil.h>
-#else
-#include <util.h>
-#endif
 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
 #include <dev/ppbus/ppi.h>
 #include <dev/ppbus/ppbconf.h>
@@ -69,8 +62,6 @@ 
 #endif
 #else
 #ifdef __linux__
-#include <pty.h>
-
 #include <linux/ppdev.h>
 #include <linux/parport.h>
 #endif
@@ -87,7 +78,6 @@ 
 #include <netinet/tcp.h>
 #include <net/if.h>
 #include <syslog.h>
-#include <stropts.h>
 #endif
 #endif
 #endif
diff --git a/ui/gtk.c b/ui/gtk.c
index e12f228..7b2d2ac 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -55,7 +55,6 @@ 
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <sys/wait.h>
-#include <pty.h>
 #include <math.h>
 
 #include "ui/console.h"