[PATCH] libxl: build fix

[PATCH] libxl: build fix

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 830 bytes --]


Fix this build error:

libxl_aoutils.c: In function 'libxl__openptys':
libxl_aoutils.c:281:13: error: passing argument 4 of 'openpty' discards
qualifiers from pointer target type
/usr/include/util.h:92:6: note: expected 'struct termios *' but argument
is of type 'const struct termios *'
libxl_aoutils.c:281:13: error: passing argument 5 of 'openpty' discards
qualifiers from pointer target type
/usr/include/util.h:92:6: note: expected 'struct winsize *' but argument
is of type 'const struct winsize *'

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_libxl.diff --]
[-- Type: text/plain, Size: 1139 bytes --]

diff -r 1463985435ba tools/libxl/libxl_aoutils.c
--- a/tools/libxl/libxl_aoutils.c	Wed May 16 17:17:17 2012 +0200
+++ b/tools/libxl/libxl_aoutils.c	Wed May 16 17:47:51 2012 +0200
@@ -230,8 +230,8 @@ static void openpty_exited(libxl__egc *e
 }
 
 int libxl__openptys(libxl__openpty_state *op,
-                    const struct termios *termp,
-                    const struct winsize *winp) {
+                    struct termios *termp,
+                    struct winsize *winp) {
     /*
      * This is completely crazy.  openpty calls grantpt which the spec
      * says may fork, and may not be called with a SIGCHLD handler.
diff -r 1463985435ba tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Wed May 16 17:17:17 2012 +0200
+++ b/tools/libxl/libxl_internal.h	Wed May 16 17:47:51 2012 +0200
@@ -1739,8 +1739,8 @@ struct libxl__openpty_result {
 };
 
 int libxl__openptys(libxl__openpty_state *op,
-                    const struct termios *termp,
-                    const struct winsize *winp);
+                    struct termios *termp,
+                    struct winsize *winp);
 
 
 /*----- bootloader -----*/

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] libxl: build fix

[Roger Pau Monne]

Christoph Egger wrote:
> Fix this build error:
>
> libxl_aoutils.c: In function 'libxl__openptys':
> libxl_aoutils.c:281:13: error: passing argument 4 of 'openpty' discards
> qualifiers from pointer target type
> /usr/include/util.h:92:6: note: expected 'struct termios *' but argument
> is of type 'const struct termios *'
> libxl_aoutils.c:281:13: error: passing argument 5 of 'openpty' discards
> qualifiers from pointer target type
> /usr/include/util.h:92:6: note: expected 'struct winsize *' but argument
> is of type 'const struct winsize *'
>
> Signed-off-by: Christoph Egger<Christoph.Egger@amd.com>

Acked-by: Roger Pau Monne <roger.pau@citrix.com>

I've tested this on both NetBSD and Debian, and it fixes the problem, 
without introducing new ones. It's strange because these are BSD 
functions, but Linux implementation change the parameters...

Re: [PATCH] libxl: build fix

[Ian Campbell]

On Wed, 2012-05-16 at 16:51 +0100, Christoph Egger wrote:
> Fix this build error:

on NetBSD I presume.

The Linux openpty(3) manpage also notes that the const was only added
there in glibc 2.8. I note that CentOS 5 has 2.5 so this would
presumably also happen on some old, but still supported, Linux distros.

So:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> 
> libxl_aoutils.c: In function 'libxl__openptys':
> libxl_aoutils.c:281:13: error: passing argument 4 of 'openpty' discards
> qualifiers from pointer target type
> /usr/include/util.h:92:6: note: expected 'struct termios *' but argument
> is of type 'const struct termios *'
> libxl_aoutils.c:281:13: error: passing argument 5 of 'openpty' discards
> qualifiers from pointer target type
> /usr/include/util.h:92:6: note: expected 'struct winsize *' but argument
> is of type 'const struct winsize *'
> 
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
> 
> 
> 

Re: [PATCH] libxl: build fix

[Ian Campbell]

On Wed, 2012-05-16 at 16:51 +0100, Christoph Egger wrote:
> Fix this build error:
> 
> libxl_aoutils.c: In function 'libxl__openptys':
> libxl_aoutils.c:281:13: error: passing argument 4 of 'openpty' discards
> qualifiers from pointer target type
> /usr/include/util.h:92:6: note: expected 'struct termios *' but argument
> is of type 'const struct termios *'
> libxl_aoutils.c:281:13: error: passing argument 5 of 'openpty' discards
> qualifiers from pointer target type
> /usr/include/util.h:92:6: note: expected 'struct winsize *' but argument
> is of type 'const struct winsize *'
> 
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

Applied, thanks.

I improved your commit message to:

   libxl: fix build on platforms where openpty's parameters are not const.
    
    Such as NetBSD. Fixes this build error:
    
        ....

Please try and make the summary line contain some useful info, it's all
you see in the default "hg log" etc.

Ian.

[PATCH] libxl: build fix

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]


Hi!

Attached patch fixes libxl build error:

libxl_dm.c:713: warning: implicit declaration of function 'kill'

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_libxl_build.diff --]
[-- Type: text/x-diff, Size: 332 bytes --]

diff -r 2007ca4414ab tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Tue Jan 11 15:34:30 2011 +0100
+++ b/tools/libxl/libxl_dm.c	Tue Jan 11 15:52:52 2011 +0100
@@ -20,6 +20,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
+#include <signal.h>
 #include <unistd.h>
 #include <fcntl.h>
 #include "libxl_utils.h"

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] libxl: build fix

[Ian Jackson]

Christoph Egger writes ("[Xen-devel] [PATCH] libxl: build fix"):
> Attached patch fixes libxl build error:
> libxl_dm.c:713: warning: implicit declaration of function 'kill'

Applied, thanks.  I see Keir applied your ctype fix too.

Ian.

[PATCH] libxl: build fix

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 473 bytes --]


Hi!

Attached patch makes libxl compile on NetBSD.
<sys/signal.h> is needed to get definition for SIGPIPE and SIG_IGN.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_libxl.diff --]
[-- Type: text/x-diff, Size: 330 bytes --]

diff -r 2a4970daad74 tools/libxl/xl.c
--- a/tools/libxl/xl.c	Tue Apr 13 09:38:54 2010 +0100
+++ b/tools/libxl/xl.c	Tue Apr 13 11:55:30 2010 +0200
@@ -26,6 +26,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
+#include <sys/signal.h>
 #include <sys/socket.h>
 #include <sys/select.h>
 #include <arpa/inet.h>

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] libxl: build fix

[Ian Jackson]

Christoph Egger writes ("[Xen-devel] [PATCH] libxl: build fix"):
> Attached patch makes libxl compile on NetBSD.
> <sys/signal.h> is needed to get definition for SIGPIPE and SIG_IGN.

<sys/signal.h> is not a standard header file.  It should be <signal.h>.

Adding
 #include <signal.h>
would be correct.

Ian.

Re: [PATCH] libxl: build fix

[Christoph Egger]

On Tuesday 13 April 2010 13:21:31 Ian Jackson wrote:
> Christoph Egger writes ("[Xen-devel] [PATCH] libxl: build fix"):
> > Attached patch makes libxl compile on NetBSD.
> > <sys/signal.h> is needed to get definition for SIGPIPE and SIG_IGN.
>
> <sys/signal.h> is not a standard header file.  It should be <signal.h>.
>
> Adding
>  #include <signal.h>
> would be correct.

On NetBSD, the SIG* defines are in <sys/signal.h> which is just
included by <signal.h>
On NetBSD, both works.

But you're right, the portable fix is the prefered one.

Christoph


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] libxl: build fix

[Ian Jackson]

Christoph Egger writes ("Re: [Xen-devel] [PATCH] libxl: build fix"):
> On NetBSD, the SIG* defines are in <sys/signal.h> which is just
> included by <signal.h>
> On NetBSD, both works.

The correct way to discover what the right #include is is not to go on
a wild goose chase through headers.  It is to read appropriate
documentation, such as SuSv3:
  http://www.opengroup.org/onlinepubs/009695399/basedefs/signal.h.html
Every function or symbol that's in SuS has next to its description a
statement of what header file(s) are needed.

> But you're right, the portable fix is the prefered one.

It's not just "portable", it is not broken.  The NetBSD developers
would be quite entitled to make <sys/signal.h> go away, or not contain
these definitions any more.  I doubt they promise not to.

Ian.

Re: [PATCH] libxl: build fix

[Christoph Egger]

On Tuesday 13 April 2010 17:04:00 Ian Jackson wrote:
> Christoph Egger writes ("Re: [Xen-devel] [PATCH] libxl: build fix"):
> > On NetBSD, the SIG* defines are in <sys/signal.h> which is just
> > included by <signal.h>
> > On NetBSD, both works.
>
> The correct way to discover what the right #include is is not to go on
> a wild goose chase through headers.  It is to read appropriate
> documentation, such as SuSv3:
>   http://www.opengroup.org/onlinepubs/009695399/basedefs/signal.h.html
> Every function or symbol that's in SuS has next to its description a
> statement of what header file(s) are needed.

Yes, you are right. It's my fault. It should have read the manpage. It states 
this clearly, too.

Christoph

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[PATCH] libxl: build fix

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 402 bytes --]


Hi!

Attached patch makes libxl build on NetBSD.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_tools_libxl.diff --]
[-- Type: text/x-diff, Size: 346 bytes --]

diff -r 030589859a8f tools/libxl/libxl_exec.c
--- a/tools/libxl/libxl_exec.c	Tue Dec 01 15:14:29 2009 +0100
+++ b/tools/libxl/libxl_exec.c	Tue Dec 01 15:40:57 2009 +0100
@@ -25,6 +25,7 @@
 #include <assert.h>
 #include <sys/types.h>
 #include <sys/wait.h>
+#include <signal.h> /* for SIGKILL */
 
 #include "libxl.h"
 #include "libxl_internal.h"

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] libxl: build fix

[Ian Jackson]

Christoph Egger writes ("[Xen-devel] [PATCH] libxl: build fix"):
> Attached patch makes libxl build on NetBSD.

The patch is fine, although I don't generally approve of comments like
the one you've included:

+#include <signal.h> /* for SIGKILL */

I would just say:

+#include <signal.h>

and you can mention SIGKILL in the commit message.


There is an argument that all of these headers should be included in
one place by libxl_internal.h.

Ian.