xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] xen/errno: Introduce EISDIR/EROFS/ENOTEMPTY to the ABI
@ 2016-03-01 18:57 Andrew Cooper
  2016-03-01 18:57 ` [PATCH 2/4] xen/errno: Declare aliases using XEN_ERRNO() Andrew Cooper
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Andrew Cooper @ 2016-03-01 18:57 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Tim Deegan, Jan Beulich, Doug Goldstein

These POSIX errnos are expected by other areas of the Xen public interface,
specifically public/io/xs_wire.h

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
CC: Doug Goldstein <cardoe@cardoe.com>
---
 xen/include/public/errno.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/include/public/errno.h b/xen/include/public/errno.h
index 8c88bb1..c3481a5 100644
--- a/xen/include/public/errno.h
+++ b/xen/include/public/errno.h
@@ -49,16 +49,19 @@ XEN_ERRNO(EBUSY,	16)	/* Device or resource busy */
 XEN_ERRNO(EEXIST,	17)	/* File exists */
 XEN_ERRNO(EXDEV,	18)	/* Cross-device link */
 XEN_ERRNO(ENODEV,	19)	/* No such device */
+XEN_ERRNO(EISDIR,	21)	/* Is a directory */
 XEN_ERRNO(EINVAL,	22)	/* Invalid argument */
 XEN_ERRNO(ENFILE,	23)	/* File table overflow */
 XEN_ERRNO(EMFILE,	24)	/* Too many open files */
 XEN_ERRNO(ENOSPC,	28)	/* No space left on device */
+XEN_ERRNO(EROFS,	30)	/* Read-only file system */
 XEN_ERRNO(EMLINK,	31)	/* Too many links */
 XEN_ERRNO(EDOM,		33)	/* Math argument out of domain of func */
 XEN_ERRNO(ERANGE,	34)	/* Math result not representable */
 XEN_ERRNO(EDEADLK,	35)	/* Resource deadlock would occur */
 XEN_ERRNO(ENAMETOOLONG,	36)	/* File name too long */
 XEN_ERRNO(ENOLCK,	37)	/* No record locks available */
+XEN_ERRNO(ENOTEMPTY,	39)	/* Directory not empty */
 XEN_ERRNO(ENOSYS,	38)	/* Function not implemented */
 XEN_ERRNO(ENODATA,	61)	/* No data available */
 XEN_ERRNO(ETIME,	62)	/* Timer expired */
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/4] xen/errno: Declare aliases using XEN_ERRNO()
  2016-03-01 18:57 [PATCH 1/4] xen/errno: Introduce EISDIR/EROFS/ENOTEMPTY to the ABI Andrew Cooper
@ 2016-03-01 18:57 ` Andrew Cooper
  2016-03-02  2:38   ` Doug Goldstein
  2016-03-01 18:57 ` [PATCH 3/4] xen/errno: Reduce complexity of inclusion Andrew Cooper
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2016-03-01 18:57 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Tim Deegan, Jan Beulich, Doug Goldstein

Otherwise a custom XEN_ERRNO definition will not end up creating appropriately
namespaced constants for the aliases.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
CC: Doug Goldstein <cardoe@cardoe.com>
---
 xen/include/public/errno.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/xen/include/public/errno.h b/xen/include/public/errno.h
index c3481a5..dbac396 100644
--- a/xen/include/public/errno.h
+++ b/xen/include/public/errno.h
@@ -42,6 +42,7 @@ XEN_ERRNO(ENOEXEC,	 8)	/* Exec format error */
 XEN_ERRNO(EBADF,	 9)	/* Bad file number */
 XEN_ERRNO(ECHILD,	10)	/* No child processes */
 XEN_ERRNO(EAGAIN,	11)	/* Try again */
+XEN_ERRNO(EWOULDBLOCK,	11)	/* Operation would block.  Aliases EAGAIN */
 XEN_ERRNO(ENOMEM,	12)	/* Out of memory */
 XEN_ERRNO(EACCES,	13)	/* Permission denied */
 XEN_ERRNO(EFAULT,	14)	/* Bad address */
@@ -59,6 +60,7 @@ XEN_ERRNO(EMLINK,	31)	/* Too many links */
 XEN_ERRNO(EDOM,		33)	/* Math argument out of domain of func */
 XEN_ERRNO(ERANGE,	34)	/* Math result not representable */
 XEN_ERRNO(EDEADLK,	35)	/* Resource deadlock would occur */
+XEN_ERRNO(EDEADLOCK,	35)	/* Resource deadlock would occur. Aliases EDEADLK */
 XEN_ERRNO(ENAMETOOLONG,	36)	/* File name too long */
 XEN_ERRNO(ENOLCK,	37)	/* No record locks available */
 XEN_ERRNO(ENOTEMPTY,	39)	/* Directory not empty */
@@ -92,7 +94,4 @@ XEN_ERRNO(ETIMEDOUT,	110)	/* Connection timed out */
 };
 #endif
 
-#define	XEN_EWOULDBLOCK	XEN_EAGAIN	/* Operation would block */
-#define	XEN_EDEADLOCK	XEN_EDEADLK	/* Resource deadlock would occur */
-
 #endif /*  __XEN_PUBLIC_ERRNO_H__ */
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/4] xen/errno: Reduce complexity of inclusion
  2016-03-01 18:57 [PATCH 1/4] xen/errno: Introduce EISDIR/EROFS/ENOTEMPTY to the ABI Andrew Cooper
  2016-03-01 18:57 ` [PATCH 2/4] xen/errno: Declare aliases using XEN_ERRNO() Andrew Cooper
@ 2016-03-01 18:57 ` Andrew Cooper
  2016-03-02  2:39   ` Doug Goldstein
  2016-03-03  8:30   ` Jan Beulich
  2016-03-01 18:57 ` [PATCH 4/4] hvmloader: Use xen/errno.h rather than the host systems errno.h Andrew Cooper
  2016-03-02  2:37 ` [PATCH 1/4] xen/errno: Introduce EISDIR/EROFS/ENOTEMPTY to the ABI Doug Goldstein
  3 siblings, 2 replies; 17+ messages in thread
From: Andrew Cooper @ 2016-03-01 18:57 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Tim Deegan, Jan Beulich, Doug Goldstein

The inclusion rules conditions for errno.h were unnecesserily complicated, and
required the includer to jump through hoops if they wished to avoid getting
multiple namespaces worth of constants.

Vastly simply the logic, and document what is going on.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
CC: Doug Goldstein <cardoe@cardoe.com>
---
 xen/include/public/errno.h | 55 ++++++++++++++++++++++++++++++----------------
 xen/include/xen/errno.h    |  6 ++---
 2 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/xen/include/public/errno.h b/xen/include/public/errno.h
index dbac396..fa375be 100644
--- a/xen/include/public/errno.h
+++ b/xen/include/public/errno.h
@@ -1,20 +1,36 @@
-#ifndef __XEN_PUBLIC_ERRNO_H__
-
-#ifndef __ASSEMBLY__
-
-#define XEN_ERRNO(name, value) XEN_##name = value,
-enum xen_errno {
+/*
+ * There are two expected ways of including this header.
+ *
+ * 1) The "default" case (expected from tools etc).
+ *
+ * Simply #include <public/errno.h>
+ *
+ * In this circumstance, normal header guards apply and the includer shall get
+ * an enumeration in the XEN_xxx namespace.
+ *
+ * 2) The special case where the includer provides a XEN_ERRNO() in scope.
+ *
+ * In this case, no inclusion guards apply and the caller is responsible for
+ * their XEN_ERRNO() being appropriate in the included context.
+ */
 
-#else /* !__ASSEMBLY__ */
+#ifndef XEN_ERRNO
 
-#define XEN_ERRNO(name, value) .equ XEN_##name, value
+/*
+ * Includer has not provided a custom XEN_ERRNO().  Arrange an automatic enum
+ * and constants in the XEN_xxx namespace.
+ */
+#define XEN_ERRNO_DEFAULT_INCLUDE
 
-#endif /* __ASSEMBLY__ */
+#ifndef __XEN_PUBLIC_ERRNO_H__
+#define __XEN_PUBLIC_ERRNO_H__
 
-/* ` enum neg_errnoval {  [ -Efoo for each Efoo in the list below ]  } */
-/* ` enum errnoval { */
+#define XEN_ERRNO(name, value) XEN_##name = value,
+enum {
 
 #endif /* __XEN_PUBLIC_ERRNO_H__ */
+#endif /* !XEN_ERRNO */
+
 
 #ifdef XEN_ERRNO
 
@@ -82,16 +98,17 @@ XEN_ERRNO(EISCONN,	106)	/* Transport endpoint is already connected */
 XEN_ERRNO(ENOTCONN,	107)	/* Transport endpoint is not connected */
 XEN_ERRNO(ETIMEDOUT,	110)	/* Connection timed out */
 
-#undef XEN_ERRNO
 #endif /* XEN_ERRNO */
 
-#ifndef __XEN_PUBLIC_ERRNO_H__
-#define __XEN_PUBLIC_ERRNO_H__
 
-/* ` } */
+#ifdef XEN_ERRNO_DEFAULT_INCLUDE
 
-#ifndef __ASSEMBLY__
-};
-#endif
+/*
+ * Clean up from a default include.  Close the enum and remove the default
+ * XEN_ERRNO from scope.
+ */
+#undef XEN_ERRNO_DEFAULT_INCLUDE
+#undef XEN_ERRNO
+} ;
 
-#endif /*  __XEN_PUBLIC_ERRNO_H__ */
+#endif /* XEN_ERRNO_DEFAULT_INCLUDE */
diff --git a/xen/include/xen/errno.h b/xen/include/xen/errno.h
index 3178466..69b28dd 100644
--- a/xen/include/xen/errno.h
+++ b/xen/include/xen/errno.h
@@ -1,18 +1,16 @@
 #ifndef __XEN_ERRNO_H__
 #define __XEN_ERRNO_H__
 
-#include <public/errno.h>
-
 #ifndef __ASSEMBLY__
 
-#define XEN_ERRNO(name, value) name = XEN_##name,
+#define XEN_ERRNO(name, value) name = value,
 enum {
 #include <public/errno.h>
 };
 
 #else /* !__ASSEMBLY__ */
 
-#define XEN_ERRNO(name, value) .equ name, XEN_##name
+#define XEN_ERRNO(name, value) .equ name, value
 #include <public/errno.h>
 
 #endif /* __ASSEMBLY__ */
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/4] hvmloader: Use xen/errno.h rather than the host systems errno.h
  2016-03-01 18:57 [PATCH 1/4] xen/errno: Introduce EISDIR/EROFS/ENOTEMPTY to the ABI Andrew Cooper
  2016-03-01 18:57 ` [PATCH 2/4] xen/errno: Declare aliases using XEN_ERRNO() Andrew Cooper
  2016-03-01 18:57 ` [PATCH 3/4] xen/errno: Reduce complexity of inclusion Andrew Cooper
@ 2016-03-01 18:57 ` Andrew Cooper
  2016-03-02 12:53   ` Doug Goldstein
  2016-03-03 11:35   ` Wei Liu
  2016-03-02  2:37 ` [PATCH 1/4] xen/errno: Introduce EISDIR/EROFS/ENOTEMPTY to the ABI Doug Goldstein
  3 siblings, 2 replies; 17+ messages in thread
From: Andrew Cooper @ 2016-03-01 18:57 UTC (permalink / raw)
  To: Xen-devel
  Cc: Wei Liu, Ian Campbell, Andrew Cooper, Doug Goldstein,
	Jan Beulich, Ian Jackson

hvmloader is unhosted, and shouldn't use the system errno.h.  It already has
to use Xen's errno.h for other hypercalls.  The use of public/io/xs_wire.h
requires the use of un-prefixed errno values.

This fixes the build on stricter toolchains where requesting -fno-builtin does
reduce the include path as much as it can.

Reported-by: Doug Goldstein <cardoe@cardoe.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Doug Goldstein <cardoe@cardoe.com>

v3:
 * Split single patch multiple fixes
v2:
 * Fix compilation.  I am not sure how v1 compiled, but I did definitely check
   it before posting.
---
 tools/firmware/hvmloader/util.h   | 9 +++++++++
 tools/firmware/hvmloader/vnuma.c  | 3 +--
 tools/firmware/hvmloader/xenbus.c | 1 -
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
index 132d915..3126817 100644
--- a/tools/firmware/hvmloader/util.h
+++ b/tools/firmware/hvmloader/util.h
@@ -9,6 +9,15 @@
 #include <xen/hvm/hvm_info_table.h>
 #include "e820.h"
 
+/* Request un-prefixed values from errno.h. */
+#define XEN_ERRNO(name, value) name = value,
+enum {
+#include <xen/errno.h>
+};
+
+/* Cause xs_wire.h to give us xsd_errors[]. */
+#define EINVAL EINVAL
+
 #define __STR(...) #__VA_ARGS__
 #define STR(...) __STR(__VA_ARGS__)
 
diff --git a/tools/firmware/hvmloader/vnuma.c b/tools/firmware/hvmloader/vnuma.c
index 4121cc6..85c1a79 100644
--- a/tools/firmware/hvmloader/vnuma.c
+++ b/tools/firmware/hvmloader/vnuma.c
@@ -28,7 +28,6 @@
 #include "util.h"
 #include "hypercall.h"
 #include "vnuma.h"
-#include <xen/errno.h>
 
 unsigned int nr_vnodes, nr_vmemranges;
 unsigned int *vcpu_to_vnode, *vdistance;
@@ -40,7 +39,7 @@ void init_vnuma_info(void)
     struct xen_vnuma_topology_info vnuma_topo = { .domid = DOMID_SELF };
 
     rc = hypercall_memory_op(XENMEM_get_vnumainfo, &vnuma_topo);
-    if ( rc != -XEN_ENOBUFS )
+    if ( rc != -ENOBUFS )
         return;
 
     ASSERT(vnuma_topo.nr_vcpus == hvm_info->nr_vcpus);
diff --git a/tools/firmware/hvmloader/xenbus.c b/tools/firmware/hvmloader/xenbus.c
index d0ed993..448157d 100644
--- a/tools/firmware/hvmloader/xenbus.c
+++ b/tools/firmware/hvmloader/xenbus.c
@@ -27,7 +27,6 @@
 
 #include "util.h"
 #include "hypercall.h"
-#include <errno.h>
 #include <xen/sched.h>
 #include <xen/event_channel.h>
 #include <xen/hvm/params.h>
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/4] xen/errno: Introduce EISDIR/EROFS/ENOTEMPTY to the ABI
  2016-03-01 18:57 [PATCH 1/4] xen/errno: Introduce EISDIR/EROFS/ENOTEMPTY to the ABI Andrew Cooper
                   ` (2 preceding siblings ...)
  2016-03-01 18:57 ` [PATCH 4/4] hvmloader: Use xen/errno.h rather than the host systems errno.h Andrew Cooper
@ 2016-03-02  2:37 ` Doug Goldstein
  3 siblings, 0 replies; 17+ messages in thread
From: Doug Goldstein @ 2016-03-02  2:37 UTC (permalink / raw)
  To: Andrew Cooper, Xen-devel; +Cc: Tim Deegan, Jan Beulich


[-- Attachment #1.1.1: Type: text/plain, Size: 1773 bytes --]

On 3/1/16 12:57 PM, Andrew Cooper wrote:
> These POSIX errnos are expected by other areas of the Xen public interface,
> specifically public/io/xs_wire.h
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Doug Goldstein <cardoe@cardoe.com>

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Tim Deegan <tim@xen.org>
> CC: Doug Goldstein <cardoe@cardoe.com>
> ---
>  xen/include/public/errno.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/xen/include/public/errno.h b/xen/include/public/errno.h
> index 8c88bb1..c3481a5 100644
> --- a/xen/include/public/errno.h
> +++ b/xen/include/public/errno.h
> @@ -49,16 +49,19 @@ XEN_ERRNO(EBUSY,	16)	/* Device or resource busy */
>  XEN_ERRNO(EEXIST,	17)	/* File exists */
>  XEN_ERRNO(EXDEV,	18)	/* Cross-device link */
>  XEN_ERRNO(ENODEV,	19)	/* No such device */
> +XEN_ERRNO(EISDIR,	21)	/* Is a directory */
>  XEN_ERRNO(EINVAL,	22)	/* Invalid argument */
>  XEN_ERRNO(ENFILE,	23)	/* File table overflow */
>  XEN_ERRNO(EMFILE,	24)	/* Too many open files */
>  XEN_ERRNO(ENOSPC,	28)	/* No space left on device */
> +XEN_ERRNO(EROFS,	30)	/* Read-only file system */
>  XEN_ERRNO(EMLINK,	31)	/* Too many links */
>  XEN_ERRNO(EDOM,		33)	/* Math argument out of domain of func */
>  XEN_ERRNO(ERANGE,	34)	/* Math result not representable */
>  XEN_ERRNO(EDEADLK,	35)	/* Resource deadlock would occur */
>  XEN_ERRNO(ENAMETOOLONG,	36)	/* File name too long */
>  XEN_ERRNO(ENOLCK,	37)	/* No record locks available */
> +XEN_ERRNO(ENOTEMPTY,	39)	/* Directory not empty */
>  XEN_ERRNO(ENOSYS,	38)	/* Function not implemented */
>  XEN_ERRNO(ENODATA,	61)	/* No data available */
>  XEN_ERRNO(ETIME,	62)	/* Timer expired */
> 


-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

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

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/4] xen/errno: Declare aliases using XEN_ERRNO()
  2016-03-01 18:57 ` [PATCH 2/4] xen/errno: Declare aliases using XEN_ERRNO() Andrew Cooper
@ 2016-03-02  2:38   ` Doug Goldstein
  0 siblings, 0 replies; 17+ messages in thread
From: Doug Goldstein @ 2016-03-02  2:38 UTC (permalink / raw)
  To: Andrew Cooper, Xen-devel; +Cc: Tim Deegan, Jan Beulich


[-- Attachment #1.1.1: Type: text/plain, Size: 1930 bytes --]

On 3/1/16 12:57 PM, Andrew Cooper wrote:
> Otherwise a custom XEN_ERRNO definition will not end up creating appropriately
> namespaced constants for the aliases.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Doug Goldstein <cardoe@cardoe.com>

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Tim Deegan <tim@xen.org>
> CC: Doug Goldstein <cardoe@cardoe.com>
> ---
>  xen/include/public/errno.h | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/include/public/errno.h b/xen/include/public/errno.h
> index c3481a5..dbac396 100644
> --- a/xen/include/public/errno.h
> +++ b/xen/include/public/errno.h
> @@ -42,6 +42,7 @@ XEN_ERRNO(ENOEXEC,	 8)	/* Exec format error */
>  XEN_ERRNO(EBADF,	 9)	/* Bad file number */
>  XEN_ERRNO(ECHILD,	10)	/* No child processes */
>  XEN_ERRNO(EAGAIN,	11)	/* Try again */
> +XEN_ERRNO(EWOULDBLOCK,	11)	/* Operation would block.  Aliases EAGAIN */
>  XEN_ERRNO(ENOMEM,	12)	/* Out of memory */
>  XEN_ERRNO(EACCES,	13)	/* Permission denied */
>  XEN_ERRNO(EFAULT,	14)	/* Bad address */
> @@ -59,6 +60,7 @@ XEN_ERRNO(EMLINK,	31)	/* Too many links */
>  XEN_ERRNO(EDOM,		33)	/* Math argument out of domain of func */
>  XEN_ERRNO(ERANGE,	34)	/* Math result not representable */
>  XEN_ERRNO(EDEADLK,	35)	/* Resource deadlock would occur */
> +XEN_ERRNO(EDEADLOCK,	35)	/* Resource deadlock would occur. Aliases EDEADLK */
>  XEN_ERRNO(ENAMETOOLONG,	36)	/* File name too long */
>  XEN_ERRNO(ENOLCK,	37)	/* No record locks available */
>  XEN_ERRNO(ENOTEMPTY,	39)	/* Directory not empty */
> @@ -92,7 +94,4 @@ XEN_ERRNO(ETIMEDOUT,	110)	/* Connection timed out */
>  };
>  #endif
>  
> -#define	XEN_EWOULDBLOCK	XEN_EAGAIN	/* Operation would block */
> -#define	XEN_EDEADLOCK	XEN_EDEADLK	/* Resource deadlock would occur */
> -
>  #endif /*  __XEN_PUBLIC_ERRNO_H__ */
> 


-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

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

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/4] xen/errno: Reduce complexity of inclusion
  2016-03-01 18:57 ` [PATCH 3/4] xen/errno: Reduce complexity of inclusion Andrew Cooper
@ 2016-03-02  2:39   ` Doug Goldstein
  2016-03-03  8:30   ` Jan Beulich
  1 sibling, 0 replies; 17+ messages in thread
From: Doug Goldstein @ 2016-03-02  2:39 UTC (permalink / raw)
  To: Andrew Cooper, Xen-devel; +Cc: Tim Deegan, Jan Beulich


[-- Attachment #1.1.1: Type: text/plain, Size: 3779 bytes --]

On 3/1/16 12:57 PM, Andrew Cooper wrote:
> The inclusion rules conditions for errno.h were unnecesserily complicated, and
> required the includer to jump through hoops if they wished to avoid getting
> multiple namespaces worth of constants.
> 
> Vastly simply the logic, and document what is going on.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Doug Goldstein <cardoe@cardoe.com>

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Tim Deegan <tim@xen.org>
> CC: Doug Goldstein <cardoe@cardoe.com>
> ---
>  xen/include/public/errno.h | 55 ++++++++++++++++++++++++++++++----------------
>  xen/include/xen/errno.h    |  6 ++---
>  2 files changed, 38 insertions(+), 23 deletions(-)
> 
> diff --git a/xen/include/public/errno.h b/xen/include/public/errno.h
> index dbac396..fa375be 100644
> --- a/xen/include/public/errno.h
> +++ b/xen/include/public/errno.h
> @@ -1,20 +1,36 @@
> -#ifndef __XEN_PUBLIC_ERRNO_H__
> -
> -#ifndef __ASSEMBLY__
> -
> -#define XEN_ERRNO(name, value) XEN_##name = value,
> -enum xen_errno {
> +/*
> + * There are two expected ways of including this header.
> + *
> + * 1) The "default" case (expected from tools etc).
> + *
> + * Simply #include <public/errno.h>
> + *
> + * In this circumstance, normal header guards apply and the includer shall get
> + * an enumeration in the XEN_xxx namespace.
> + *
> + * 2) The special case where the includer provides a XEN_ERRNO() in scope.
> + *
> + * In this case, no inclusion guards apply and the caller is responsible for
> + * their XEN_ERRNO() being appropriate in the included context.
> + */
>  
> -#else /* !__ASSEMBLY__ */
> +#ifndef XEN_ERRNO
>  
> -#define XEN_ERRNO(name, value) .equ XEN_##name, value
> +/*
> + * Includer has not provided a custom XEN_ERRNO().  Arrange an automatic enum
> + * and constants in the XEN_xxx namespace.
> + */
> +#define XEN_ERRNO_DEFAULT_INCLUDE
>  
> -#endif /* __ASSEMBLY__ */
> +#ifndef __XEN_PUBLIC_ERRNO_H__
> +#define __XEN_PUBLIC_ERRNO_H__
>  
> -/* ` enum neg_errnoval {  [ -Efoo for each Efoo in the list below ]  } */
> -/* ` enum errnoval { */
> +#define XEN_ERRNO(name, value) XEN_##name = value,
> +enum {
>  
>  #endif /* __XEN_PUBLIC_ERRNO_H__ */
> +#endif /* !XEN_ERRNO */
> +
>  
>  #ifdef XEN_ERRNO
>  
> @@ -82,16 +98,17 @@ XEN_ERRNO(EISCONN,	106)	/* Transport endpoint is already connected */
>  XEN_ERRNO(ENOTCONN,	107)	/* Transport endpoint is not connected */
>  XEN_ERRNO(ETIMEDOUT,	110)	/* Connection timed out */
>  
> -#undef XEN_ERRNO
>  #endif /* XEN_ERRNO */
>  
> -#ifndef __XEN_PUBLIC_ERRNO_H__
> -#define __XEN_PUBLIC_ERRNO_H__
>  
> -/* ` } */
> +#ifdef XEN_ERRNO_DEFAULT_INCLUDE
>  
> -#ifndef __ASSEMBLY__
> -};
> -#endif
> +/*
> + * Clean up from a default include.  Close the enum and remove the default
> + * XEN_ERRNO from scope.
> + */
> +#undef XEN_ERRNO_DEFAULT_INCLUDE
> +#undef XEN_ERRNO
> +} ;
>  
> -#endif /*  __XEN_PUBLIC_ERRNO_H__ */
> +#endif /* XEN_ERRNO_DEFAULT_INCLUDE */
> diff --git a/xen/include/xen/errno.h b/xen/include/xen/errno.h
> index 3178466..69b28dd 100644
> --- a/xen/include/xen/errno.h
> +++ b/xen/include/xen/errno.h
> @@ -1,18 +1,16 @@
>  #ifndef __XEN_ERRNO_H__
>  #define __XEN_ERRNO_H__
>  
> -#include <public/errno.h>
> -
>  #ifndef __ASSEMBLY__
>  
> -#define XEN_ERRNO(name, value) name = XEN_##name,
> +#define XEN_ERRNO(name, value) name = value,
>  enum {
>  #include <public/errno.h>
>  };
>  
>  #else /* !__ASSEMBLY__ */
>  
> -#define XEN_ERRNO(name, value) .equ name, XEN_##name
> +#define XEN_ERRNO(name, value) .equ name, value
>  #include <public/errno.h>
>  
>  #endif /* __ASSEMBLY__ */
> 


-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

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

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] hvmloader: Use xen/errno.h rather than the host systems errno.h
  2016-03-01 18:57 ` [PATCH 4/4] hvmloader: Use xen/errno.h rather than the host systems errno.h Andrew Cooper
@ 2016-03-02 12:53   ` Doug Goldstein
  2016-03-03 11:35   ` Wei Liu
  1 sibling, 0 replies; 17+ messages in thread
From: Doug Goldstein @ 2016-03-02 12:53 UTC (permalink / raw)
  To: Andrew Cooper, Xen-devel; +Cc: Wei Liu, Ian Jackson, Ian Campbell, Jan Beulich


[-- Attachment #1.1.1: Type: text/plain, Size: 2925 bytes --]

On 3/1/16 12:57 PM, Andrew Cooper wrote:
> hvmloader is unhosted, and shouldn't use the system errno.h.  It already has
> to use Xen's errno.h for other hypercalls.  The use of public/io/xs_wire.h
> requires the use of un-prefixed errno values.
> 
> This fixes the build on stricter toolchains where requesting -fno-builtin does
> reduce the include path as much as it can.
> 
> Reported-by: Doug Goldstein <cardoe@cardoe.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Doug Goldstein <cardoe@cardoe.com>

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Doug Goldstein <cardoe@cardoe.com>
> 
> v3:
>  * Split single patch multiple fixes
> v2:
>  * Fix compilation.  I am not sure how v1 compiled, but I did definitely check
>    it before posting.
> ---
>  tools/firmware/hvmloader/util.h   | 9 +++++++++
>  tools/firmware/hvmloader/vnuma.c  | 3 +--
>  tools/firmware/hvmloader/xenbus.c | 1 -
>  3 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
> index 132d915..3126817 100644
> --- a/tools/firmware/hvmloader/util.h
> +++ b/tools/firmware/hvmloader/util.h
> @@ -9,6 +9,15 @@
>  #include <xen/hvm/hvm_info_table.h>
>  #include "e820.h"
>  
> +/* Request un-prefixed values from errno.h. */
> +#define XEN_ERRNO(name, value) name = value,
> +enum {
> +#include <xen/errno.h>
> +};
> +
> +/* Cause xs_wire.h to give us xsd_errors[]. */
> +#define EINVAL EINVAL
> +
>  #define __STR(...) #__VA_ARGS__
>  #define STR(...) __STR(__VA_ARGS__)
>  
> diff --git a/tools/firmware/hvmloader/vnuma.c b/tools/firmware/hvmloader/vnuma.c
> index 4121cc6..85c1a79 100644
> --- a/tools/firmware/hvmloader/vnuma.c
> +++ b/tools/firmware/hvmloader/vnuma.c
> @@ -28,7 +28,6 @@
>  #include "util.h"
>  #include "hypercall.h"
>  #include "vnuma.h"
> -#include <xen/errno.h>
>  
>  unsigned int nr_vnodes, nr_vmemranges;
>  unsigned int *vcpu_to_vnode, *vdistance;
> @@ -40,7 +39,7 @@ void init_vnuma_info(void)
>      struct xen_vnuma_topology_info vnuma_topo = { .domid = DOMID_SELF };
>  
>      rc = hypercall_memory_op(XENMEM_get_vnumainfo, &vnuma_topo);
> -    if ( rc != -XEN_ENOBUFS )
> +    if ( rc != -ENOBUFS )
>          return;
>  
>      ASSERT(vnuma_topo.nr_vcpus == hvm_info->nr_vcpus);
> diff --git a/tools/firmware/hvmloader/xenbus.c b/tools/firmware/hvmloader/xenbus.c
> index d0ed993..448157d 100644
> --- a/tools/firmware/hvmloader/xenbus.c
> +++ b/tools/firmware/hvmloader/xenbus.c
> @@ -27,7 +27,6 @@
>  
>  #include "util.h"
>  #include "hypercall.h"
> -#include <errno.h>
>  #include <xen/sched.h>
>  #include <xen/event_channel.h>
>  #include <xen/hvm/params.h>
> 


-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

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

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/4] xen/errno: Reduce complexity of inclusion
  2016-03-01 18:57 ` [PATCH 3/4] xen/errno: Reduce complexity of inclusion Andrew Cooper
  2016-03-02  2:39   ` Doug Goldstein
@ 2016-03-03  8:30   ` Jan Beulich
  2016-03-03 14:14     ` [PATCH v3] " Andrew Cooper
  1 sibling, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2016-03-03  8:30 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Tim Deegan, Doug Goldstein, Xen-devel

>>> On 01.03.16 at 19:57, <andrew.cooper3@citrix.com> wrote:
> The inclusion rules conditions for errno.h were unnecesserily complicated, and
> required the includer to jump through hoops if they wished to avoid getting
> multiple namespaces worth of constants.
> 
> Vastly simply the logic, and document what is going on.

Nice. While the enforcement of the creation of the XEN_* variants
was intentional, you're right that it's not really necessary.

> --- a/xen/include/public/errno.h
> +++ b/xen/include/public/errno.h
> @@ -1,20 +1,36 @@
> -#ifndef __XEN_PUBLIC_ERRNO_H__
> -
> -#ifndef __ASSEMBLY__
> -
> -#define XEN_ERRNO(name, value) XEN_##name = value,
> -enum xen_errno {
> +/*
> + * There are two expected ways of including this header.
> + *
> + * 1) The "default" case (expected from tools etc).
> + *
> + * Simply #include <public/errno.h>
> + *
> + * In this circumstance, normal header guards apply and the includer shall get
> + * an enumeration in the XEN_xxx namespace.
> + *
> + * 2) The special case where the includer provides a XEN_ERRNO() in scope.
> + *
> + * In this case, no inclusion guards apply and the caller is responsible for
> + * their XEN_ERRNO() being appropriate in the included context.
> + */
>  
> -#else /* !__ASSEMBLY__ */
> +#ifndef XEN_ERRNO
>  
> -#define XEN_ERRNO(name, value) .equ XEN_##name, value
> +/*
> + * Includer has not provided a custom XEN_ERRNO().  Arrange an automatic enum
> + * and constants in the XEN_xxx namespace.
> + */
> +#define XEN_ERRNO_DEFAULT_INCLUDE
>  
> -#endif /* __ASSEMBLY__ */
> +#ifndef __XEN_PUBLIC_ERRNO_H__
> +#define __XEN_PUBLIC_ERRNO_H__
>  
> -/* ` enum neg_errnoval {  [ -Efoo for each Efoo in the list below ]  } */
> -/* ` enum errnoval { */
> +#define XEN_ERRNO(name, value) XEN_##name = value,
> +enum {
>  
>  #endif /* __XEN_PUBLIC_ERRNO_H__ */
> +#endif /* !XEN_ERRNO */
> +
>  
>  #ifdef XEN_ERRNO

This eliminates the default creation of equates in the assembly
case, which - with the header being part of 4.6.x - we can't do.

> @@ -82,16 +98,17 @@ XEN_ERRNO(EISCONN,	106)	/* Transport endpoint is already 
> connected */
>  XEN_ERRNO(ENOTCONN,	107)	/* Transport endpoint is not connected */
>  XEN_ERRNO(ETIMEDOUT,	110)	/* Connection timed out */
>  
> -#undef XEN_ERRNO
>  #endif /* XEN_ERRNO */
>  
> -#ifndef __XEN_PUBLIC_ERRNO_H__
> -#define __XEN_PUBLIC_ERRNO_H__
>  
> -/* ` } */
> +#ifdef XEN_ERRNO_DEFAULT_INCLUDE
>  
> -#ifndef __ASSEMBLY__
> -};
> -#endif
> +/*
> + * Clean up from a default include.  Close the enum and remove the default
> + * XEN_ERRNO from scope.
> + */
> +#undef XEN_ERRNO_DEFAULT_INCLUDE
> +#undef XEN_ERRNO
> +} ;

Stray blank.

Jan


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] hvmloader: Use xen/errno.h rather than the host systems errno.h
  2016-03-01 18:57 ` [PATCH 4/4] hvmloader: Use xen/errno.h rather than the host systems errno.h Andrew Cooper
  2016-03-02 12:53   ` Doug Goldstein
@ 2016-03-03 11:35   ` Wei Liu
  1 sibling, 0 replies; 17+ messages in thread
From: Wei Liu @ 2016-03-03 11:35 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: Wei Liu, Ian Campbell, Ian Jackson, Doug Goldstein, Xen-devel,
	Jan Beulich

On Tue, Mar 01, 2016 at 06:57:21PM +0000, Andrew Cooper wrote:
> hvmloader is unhosted, and shouldn't use the system errno.h.  It already has
> to use Xen's errno.h for other hypercalls.  The use of public/io/xs_wire.h
> requires the use of un-prefixed errno values.
> 
> This fixes the build on stricter toolchains where requesting -fno-builtin does
> reduce the include path as much as it can.
> 
> Reported-by: Doug Goldstein <cardoe@cardoe.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v3] xen/errno: Reduce complexity of inclusion
  2016-03-03  8:30   ` Jan Beulich
@ 2016-03-03 14:14     ` Andrew Cooper
  2016-03-04 12:24       ` Jan Beulich
  0 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2016-03-03 14:14 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Tim Deegan, Jan Beulich, Doug Goldstein

The inclusion rules conditions for errno.h were unnecesserily complicated, and
required the includer to jump through hoops if they wished to avoid getting
multiple namespaces worth of constants.

Simply the logic, and document what is going on.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
CC: Doug Goldstein <cardoe@cardoe.com>

v3:
 * Reinstate magic documentation comments
 * Provide assembly-suitable defaults if appropriate
---
 xen/include/public/errno.h | 46 ++++++++++++++++++++++++++++++++++++++--------
 xen/include/xen/errno.h    |  6 ++----
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/xen/include/public/errno.h b/xen/include/public/errno.h
index dbac396..88e72a8 100644
--- a/xen/include/public/errno.h
+++ b/xen/include/public/errno.h
@@ -1,4 +1,30 @@
+/*
+ * There are two expected ways of including this header.
+ *
+ * 1) The "default" case (expected from tools etc).
+ *
+ * Simply #include <public/errno.h>
+ *
+ * In this circumstance, normal header guards apply and the includer shall get
+ * an enumeration in the XEN_xxx namespace, appropriate for C or assembly.
+ *
+ * 2) The special case where the includer provides a XEN_ERRNO() in scope.
+ *
+ * In this case, no inclusion guards apply and the caller is responsible for
+ * their XEN_ERRNO() being appropriate in the included context.
+ */
+
+#ifndef XEN_ERRNO
+
+/*
+ * Includer has not provided a custom XEN_ERRNO().  Arrange for normal header
+ * guards, an automatic enum (for C code) and constants in the XEN_xxx
+ * namespace.
+ */
 #ifndef __XEN_PUBLIC_ERRNO_H__
+#define __XEN_PUBLIC_ERRNO_H__
+
+#define XEN_ERRNO_DEFAULT_INCLUDE
 
 #ifndef __ASSEMBLY__
 
@@ -11,11 +37,12 @@ enum xen_errno {
 
 #endif /* __ASSEMBLY__ */
 
+#endif /* __XEN_PUBLIC_ERRNO_H__ */
+#endif /* !XEN_ERRNO */
+
 /* ` enum neg_errnoval {  [ -Efoo for each Efoo in the list below ]  } */
 /* ` enum errnoval { */
 
-#endif /* __XEN_PUBLIC_ERRNO_H__ */
-
 #ifdef XEN_ERRNO
 
 /*
@@ -82,16 +109,19 @@ XEN_ERRNO(EISCONN,	106)	/* Transport endpoint is already connected */
 XEN_ERRNO(ENOTCONN,	107)	/* Transport endpoint is not connected */
 XEN_ERRNO(ETIMEDOUT,	110)	/* Connection timed out */
 
-#undef XEN_ERRNO
 #endif /* XEN_ERRNO */
-
-#ifndef __XEN_PUBLIC_ERRNO_H__
-#define __XEN_PUBLIC_ERRNO_H__
-
 /* ` } */
 
+
+/*
+ * Clean up from a default include.  Close the enum (for C) and remove the
+ * default XEN_ERRNO from scope.
+ */
+#ifdef XEN_ERRNO_DEFAULT_INCLUDE
+#undef XEN_ERRNO_DEFAULT_INCLUDE
+#undef XEN_ERRNO
 #ifndef __ASSEMBLY__
 };
 #endif
 
-#endif /*  __XEN_PUBLIC_ERRNO_H__ */
+#endif /* XEN_ERRNO_DEFAULT_INCLUDE */
diff --git a/xen/include/xen/errno.h b/xen/include/xen/errno.h
index 3178466..69b28dd 100644
--- a/xen/include/xen/errno.h
+++ b/xen/include/xen/errno.h
@@ -1,18 +1,16 @@
 #ifndef __XEN_ERRNO_H__
 #define __XEN_ERRNO_H__
 
-#include <public/errno.h>
-
 #ifndef __ASSEMBLY__
 
-#define XEN_ERRNO(name, value) name = XEN_##name,
+#define XEN_ERRNO(name, value) name = value,
 enum {
 #include <public/errno.h>
 };
 
 #else /* !__ASSEMBLY__ */
 
-#define XEN_ERRNO(name, value) .equ name, XEN_##name
+#define XEN_ERRNO(name, value) .equ name, value
 #include <public/errno.h>
 
 #endif /* __ASSEMBLY__ */
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] xen/errno: Reduce complexity of inclusion
  2016-03-03 14:14     ` [PATCH v3] " Andrew Cooper
@ 2016-03-04 12:24       ` Jan Beulich
  2016-03-04 12:28         ` Jan Beulich
  0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2016-03-04 12:24 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Tim Deegan, Doug Goldstein, Xen-devel

>>> On 03.03.16 at 15:14, <andrew.cooper3@citrix.com> wrote:
> @@ -82,16 +109,19 @@ XEN_ERRNO(EISCONN,	106)	/* Transport endpoint is already connected */
>  XEN_ERRNO(ENOTCONN,	107)	/* Transport endpoint is not connected */
>  XEN_ERRNO(ETIMEDOUT,	110)	/* Connection timed out */
>  
> -#undef XEN_ERRNO
>  #endif /* XEN_ERRNO */
> -
> -#ifndef __XEN_PUBLIC_ERRNO_H__
> -#define __XEN_PUBLIC_ERRNO_H__
> -
>  /* ` } */
>  
> +
> +/*
> + * Clean up from a default include.  Close the enum (for C) and remove the
> + * default XEN_ERRNO from scope.
> + */
> +#ifdef XEN_ERRNO_DEFAULT_INCLUDE
> +#undef XEN_ERRNO_DEFAULT_INCLUDE
> +#undef XEN_ERRNO
>  #ifndef __ASSEMBLY__
>  };
>  #endif
>  
> -#endif /*  __XEN_PUBLIC_ERRNO_H__ */
> +#endif /* XEN_ERRNO_DEFAULT_INCLUDE */

So far, upon reaching the end oft the file XEN_ERRNO is undefined,
no matter whether it got defined here or prior to inclusion. I think
this property should be retained, but moving the #undef to the
very end. Please indicate if that's okay with you, as this doesn't
really require another version to be sent.

Jan


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] xen/errno: Reduce complexity of inclusion
  2016-03-04 12:24       ` Jan Beulich
@ 2016-03-04 12:28         ` Jan Beulich
  2016-03-04 12:50           ` Andrew Cooper
  0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2016-03-04 12:28 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Tim Deegan, Doug Goldstein, Xen-devel

>>> On 04.03.16 at 13:24, <JBeulich@suse.com> wrote:
>>>> On 03.03.16 at 15:14, <andrew.cooper3@citrix.com> wrote:
>> @@ -82,16 +109,19 @@ XEN_ERRNO(EISCONN,	106)	/* Transport endpoint is already 
> connected */
>>  XEN_ERRNO(ENOTCONN,	107)	/* Transport endpoint is not connected */
>>  XEN_ERRNO(ETIMEDOUT,	110)	/* Connection timed out */
>>  
>> -#undef XEN_ERRNO
>>  #endif /* XEN_ERRNO */
>> -
>> -#ifndef __XEN_PUBLIC_ERRNO_H__
>> -#define __XEN_PUBLIC_ERRNO_H__
>> -
>>  /* ` } */
>>  
>> +
>> +/*
>> + * Clean up from a default include.  Close the enum (for C) and remove the
>> + * default XEN_ERRNO from scope.
>> + */
>> +#ifdef XEN_ERRNO_DEFAULT_INCLUDE
>> +#undef XEN_ERRNO_DEFAULT_INCLUDE
>> +#undef XEN_ERRNO
>>  #ifndef __ASSEMBLY__
>>  };
>>  #endif
>>  
>> -#endif /*  __XEN_PUBLIC_ERRNO_H__ */
>> +#endif /* XEN_ERRNO_DEFAULT_INCLUDE */
> 
> So far, upon reaching the end oft the file XEN_ERRNO is undefined,
> no matter whether it got defined here or prior to inclusion. I think
> this property should be retained, but moving the #undef to the
> very end.

Or rather not removing it from where it's now.

Jan


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] xen/errno: Reduce complexity of inclusion
  2016-03-04 12:28         ` Jan Beulich
@ 2016-03-04 12:50           ` Andrew Cooper
  2016-03-04 13:05             ` Jan Beulich
  0 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2016-03-04 12:50 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Tim Deegan, Doug Goldstein, Xen-devel

On 04/03/16 12:28, Jan Beulich wrote:
>>>> On 04.03.16 at 13:24, <JBeulich@suse.com> wrote:
>>>>> On 03.03.16 at 15:14, <andrew.cooper3@citrix.com> wrote:
>>> @@ -82,16 +109,19 @@ XEN_ERRNO(EISCONN,	106)	/* Transport endpoint is already 
>> connected */
>>>  XEN_ERRNO(ENOTCONN,	107)	/* Transport endpoint is not connected */
>>>  XEN_ERRNO(ETIMEDOUT,	110)	/* Connection timed out */
>>>  
>>> -#undef XEN_ERRNO
>>>  #endif /* XEN_ERRNO */
>>> -
>>> -#ifndef __XEN_PUBLIC_ERRNO_H__
>>> -#define __XEN_PUBLIC_ERRNO_H__
>>> -
>>>  /* ` } */
>>>  
>>> +
>>> +/*
>>> + * Clean up from a default include.  Close the enum (for C) and remove the
>>> + * default XEN_ERRNO from scope.
>>> + */
>>> +#ifdef XEN_ERRNO_DEFAULT_INCLUDE
>>> +#undef XEN_ERRNO_DEFAULT_INCLUDE
>>> +#undef XEN_ERRNO
>>>  #ifndef __ASSEMBLY__
>>>  };
>>>  #endif
>>>  
>>> -#endif /*  __XEN_PUBLIC_ERRNO_H__ */
>>> +#endif /* XEN_ERRNO_DEFAULT_INCLUDE */
>> So far, upon reaching the end oft the file XEN_ERRNO is undefined,
>> no matter whether it got defined here or prior to inclusion. I think
>> this property should be retained, but moving the #undef to the
>> very end.
> Or rather not removing it from where it's now.

IMO, it is wrong to undef XEN_ERRNO if it was provided from external scope.

The only users of this "custom" include in Xen itself, and shortly,
hvmloader.

I think it is fine as-is.

~Andrew

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] xen/errno: Reduce complexity of inclusion
  2016-03-04 12:50           ` Andrew Cooper
@ 2016-03-04 13:05             ` Jan Beulich
  2016-03-04 16:07               ` Ian Jackson
  0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2016-03-04 13:05 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Tim Deegan, Doug Goldstein, Xen-devel

>>> On 04.03.16 at 13:50, <andrew.cooper3@citrix.com> wrote:
> On 04/03/16 12:28, Jan Beulich wrote:
>>>>> On 04.03.16 at 13:24, <JBeulich@suse.com> wrote:
>>>>>> On 03.03.16 at 15:14, <andrew.cooper3@citrix.com> wrote:
>>>> @@ -82,16 +109,19 @@ XEN_ERRNO(EISCONN,	106)	/* Transport endpoint is already 
>>> connected */
>>>>  XEN_ERRNO(ENOTCONN,	107)	/* Transport endpoint is not connected */
>>>>  XEN_ERRNO(ETIMEDOUT,	110)	/* Connection timed out */
>>>>  
>>>> -#undef XEN_ERRNO
>>>>  #endif /* XEN_ERRNO */
>>>> -
>>>> -#ifndef __XEN_PUBLIC_ERRNO_H__
>>>> -#define __XEN_PUBLIC_ERRNO_H__
>>>> -
>>>>  /* ` } */
>>>>  
>>>> +
>>>> +/*
>>>> + * Clean up from a default include.  Close the enum (for C) and remove the
>>>> + * default XEN_ERRNO from scope.
>>>> + */
>>>> +#ifdef XEN_ERRNO_DEFAULT_INCLUDE
>>>> +#undef XEN_ERRNO_DEFAULT_INCLUDE
>>>> +#undef XEN_ERRNO
>>>>  #ifndef __ASSEMBLY__
>>>>  };
>>>>  #endif
>>>>  
>>>> -#endif /*  __XEN_PUBLIC_ERRNO_H__ */
>>>> +#endif /* XEN_ERRNO_DEFAULT_INCLUDE */
>>> So far, upon reaching the end oft the file XEN_ERRNO is undefined,
>>> no matter whether it got defined here or prior to inclusion. I think
>>> this property should be retained, but moving the #undef to the
>>> very end.
>> Or rather not removing it from where it's now.
> 
> IMO, it is wrong to undef XEN_ERRNO if it was provided from external scope.

Well, even if not written down, that's the intended behavior: The
use case for the including file to further need that definition is
rather hard to see, whereas use cases for the including file
wanting to do multiple inclusion are quite easy to construct, and
in that case the including file would needlessly be forced to
repeatedly #undef the symbol.

> The only users of this "custom" include in Xen itself, and shortly,
> hvmloader.

The only users you know of. I'm definitely having plans to make
Linux use it too, to have a build time check that the errno values
continue to be in sync.

> I think it is fine as-is.

I could use the same words, just for the unpatched file: I don't
really agree with this part of your change.

Jan


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] xen/errno: Reduce complexity of inclusion
  2016-03-04 13:05             ` Jan Beulich
@ 2016-03-04 16:07               ` Ian Jackson
  2016-03-07 15:13                 ` [PATCH v4] " Andrew Cooper
  0 siblings, 1 reply; 17+ messages in thread
From: Ian Jackson @ 2016-03-04 16:07 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, Tim Deegan, Doug Goldstein, Xen-devel

Jan Beulich writes ("Re: [Xen-devel] [PATCH v3] xen/errno: Reduce complexity of inclusion"):
> On 04.03.16 at 13:50, <andrew.cooper3@citrix.com> wrote:
> > IMO, it is wrong to undef XEN_ERRNO if it was provided from external scope.
> 
> Well, even if not written down, that's the intended behavior: The
> use case for the including file to further need that definition is
> rather hard to see, whereas use cases for the including file
> wanting to do multiple inclusion are quite easy to construct, and
> in that case the including file would needlessly be forced to
> repeatedly #undef the symbol.

This is really a very bikeshed issue, but: I slightly prefer Jan's
argument here to Andrew's.

I think that this overall patch from Andrew is very helpful and I
would like to see this minor issue resolved.

Andrew, would you be able to revise the header to #undef XEN_ERRNO in
this case ?  Naturally this would want to be documented, in your
admirable new doc comment.

Thanks,
Ian.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v4] xen/errno: Reduce complexity of inclusion
  2016-03-04 16:07               ` Ian Jackson
@ 2016-03-07 15:13                 ` Andrew Cooper
  0 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2016-03-07 15:13 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Tim Deegan, Jan Beulich, Doug Goldstein

The inclusion rules conditions for errno.h were unnecesserily complicated, and
required the includer to jump through hoops if they wished to avoid getting
multiple namespaces worth of constants.

Simply the logic, and document what is going on.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
CC: Doug Goldstein <cardoe@cardoe.com>

v3:
 * Reinstate magic documentation comments
 * Provide assembly-suitable defaults if appropriate

v4:
 * Reintroduce broken #undef logic to be bug-compatible with the previous
   version.
---
 xen/include/public/errno.h | 41 ++++++++++++++++++++++++++++++++++-------
 xen/include/xen/errno.h    |  6 ++----
 2 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/xen/include/public/errno.h b/xen/include/public/errno.h
index dbac396..a0dd0cf 100644
--- a/xen/include/public/errno.h
+++ b/xen/include/public/errno.h
@@ -1,4 +1,31 @@
+/*
+ * There are two expected ways of including this header.
+ *
+ * 1) The "default" case (expected from tools etc).
+ *
+ * Simply #include <public/errno.h>
+ *
+ * In this circumstance, normal header guards apply and the includer shall get
+ * an enumeration in the XEN_xxx namespace, appropriate for C or assembly.
+ *
+ * 2) The special case where the includer provides a XEN_ERRNO() in scope.
+ *
+ * In this case, no inclusion guards apply and the caller is responsible for
+ * their XEN_ERRNO() being appropriate in the included context.  The header
+ * will unilaterally #undef XEN_ERRNO().
+ */
+
+#ifndef XEN_ERRNO
+
+/*
+ * Includer has not provided a custom XEN_ERRNO().  Arrange for normal header
+ * guards, an automatic enum (for C code) and constants in the XEN_xxx
+ * namespace.
+ */
 #ifndef __XEN_PUBLIC_ERRNO_H__
+#define __XEN_PUBLIC_ERRNO_H__
+
+#define XEN_ERRNO_DEFAULT_INCLUDE
 
 #ifndef __ASSEMBLY__
 
@@ -11,11 +38,12 @@ enum xen_errno {
 
 #endif /* __ASSEMBLY__ */
 
+#endif /* __XEN_PUBLIC_ERRNO_H__ */
+#endif /* !XEN_ERRNO */
+
 /* ` enum neg_errnoval {  [ -Efoo for each Efoo in the list below ]  } */
 /* ` enum errnoval { */
 
-#endif /* __XEN_PUBLIC_ERRNO_H__ */
-
 #ifdef XEN_ERRNO
 
 /*
@@ -84,14 +112,13 @@ XEN_ERRNO(ETIMEDOUT,	110)	/* Connection timed out */
 
 #undef XEN_ERRNO
 #endif /* XEN_ERRNO */
-
-#ifndef __XEN_PUBLIC_ERRNO_H__
-#define __XEN_PUBLIC_ERRNO_H__
-
 /* ` } */
 
+/* Clean up from a default include.  Close the enum (for C). */
+#ifdef XEN_ERRNO_DEFAULT_INCLUDE
+#undef XEN_ERRNO_DEFAULT_INCLUDE
 #ifndef __ASSEMBLY__
 };
 #endif
 
-#endif /*  __XEN_PUBLIC_ERRNO_H__ */
+#endif /* XEN_ERRNO_DEFAULT_INCLUDE */
diff --git a/xen/include/xen/errno.h b/xen/include/xen/errno.h
index 3178466..69b28dd 100644
--- a/xen/include/xen/errno.h
+++ b/xen/include/xen/errno.h
@@ -1,18 +1,16 @@
 #ifndef __XEN_ERRNO_H__
 #define __XEN_ERRNO_H__
 
-#include <public/errno.h>
-
 #ifndef __ASSEMBLY__
 
-#define XEN_ERRNO(name, value) name = XEN_##name,
+#define XEN_ERRNO(name, value) name = value,
 enum {
 #include <public/errno.h>
 };
 
 #else /* !__ASSEMBLY__ */
 
-#define XEN_ERRNO(name, value) .equ name, XEN_##name
+#define XEN_ERRNO(name, value) .equ name, value
 #include <public/errno.h>
 
 #endif /* __ASSEMBLY__ */
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-03-07 15:13 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-01 18:57 [PATCH 1/4] xen/errno: Introduce EISDIR/EROFS/ENOTEMPTY to the ABI Andrew Cooper
2016-03-01 18:57 ` [PATCH 2/4] xen/errno: Declare aliases using XEN_ERRNO() Andrew Cooper
2016-03-02  2:38   ` Doug Goldstein
2016-03-01 18:57 ` [PATCH 3/4] xen/errno: Reduce complexity of inclusion Andrew Cooper
2016-03-02  2:39   ` Doug Goldstein
2016-03-03  8:30   ` Jan Beulich
2016-03-03 14:14     ` [PATCH v3] " Andrew Cooper
2016-03-04 12:24       ` Jan Beulich
2016-03-04 12:28         ` Jan Beulich
2016-03-04 12:50           ` Andrew Cooper
2016-03-04 13:05             ` Jan Beulich
2016-03-04 16:07               ` Ian Jackson
2016-03-07 15:13                 ` [PATCH v4] " Andrew Cooper
2016-03-01 18:57 ` [PATCH 4/4] hvmloader: Use xen/errno.h rather than the host systems errno.h Andrew Cooper
2016-03-02 12:53   ` Doug Goldstein
2016-03-03 11:35   ` Wei Liu
2016-03-02  2:37 ` [PATCH 1/4] xen/errno: Introduce EISDIR/EROFS/ENOTEMPTY to the ABI Doug Goldstein

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).