* [PATCH v3 1/3] xen/string: Clean up {xen, arm}/string.h
2017-05-12 17:35 [PATCH v3 for-next 0/3] Improvements to string.h Andrew Cooper
@ 2017-05-12 17:35 ` Andrew Cooper
2017-05-15 10:02 ` Jan Beulich
2017-05-31 8:47 ` Julien Grall
2017-05-12 17:35 ` [PATCH v3 2/3] xen/string: Use compiler __builtin_*() where possible Andrew Cooper
2017-05-12 17:35 ` [PATCH v3 3/3] x86/string: Clean up x86/string.h Andrew Cooper
2 siblings, 2 replies; 13+ messages in thread
From: Andrew Cooper @ 2017-05-12 17:35 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Jan Beulich
* Drop __kernel_size_t entirely. It isn't useful distinction, especially as
it means the the prototypes don't appear to match their common definitions.
* Introduce __HAVE_ARCH_* guards for strpbrk(), strsep() and strspn(), which
match their implementation in common/string.c
* Apply consistent Xen style throughout.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
v3:
* New
---
xen/include/asm-arm/string.h | 27 +++++++------
xen/include/xen/string.h | 91 +++++++++++++++++++++++++++-----------------
2 files changed, 69 insertions(+), 49 deletions(-)
diff --git a/xen/include/asm-arm/string.h b/xen/include/asm-arm/string.h
index 005959f..1804c3c 100644
--- a/xen/include/asm-arm/string.h
+++ b/xen/include/asm-arm/string.h
@@ -8,48 +8,47 @@
*/
#define __HAVE_ARCH_STRRCHR
-extern char * strrchr(const char * s, int c);
+char *strrchr(const char *s, int c);
#define __HAVE_ARCH_STRCHR
-extern char * strchr(const char * s, int c);
+char *strchr(const char *s, int c);
#if defined(CONFIG_ARM_64)
#define __HAVE_ARCH_STRCMP
-extern int strcmp(const char *, const char *);
+int strcmp(const char *, const char *);
#define __HAVE_ARCH_STRNCMP
-extern int strncmp(const char *, const char *, __kernel_size_t);
+int strncmp(const char *, const char *, size_t);
#define __HAVE_ARCH_STRLEN
-extern __kernel_size_t strlen(const char *);
+size_t strlen(const char *);
#define __HAVE_ARCH_STRNLEN
-extern __kernel_size_t strnlen(const char *, __kernel_size_t);
+size_t strnlen(const char *, size_t);
#endif
#define __HAVE_ARCH_MEMCPY
-extern void * memcpy(void *, const void *, __kernel_size_t);
+void *memcpy(void *, const void *, size_t);
#if defined(CONFIG_ARM_64)
#define __HAVE_ARCH_MEMCMP
-extern int memcmp(const void *, const void *, __kernel_size_t);
+int memcmp(const void *, const void *, size_t);
#endif
-/* Some versions of gcc don't have this builtin. It's non-critical anyway. */
#define __HAVE_ARCH_MEMMOVE
-extern void *memmove(void *dest, const void *src, size_t n);
+void *memmove(void *dest, const void *src, size_t n);
#define __HAVE_ARCH_MEMSET
-extern void * memset(void *, int, __kernel_size_t);
+void *memset(void *, int, size_t);
#define __HAVE_ARCH_MEMCHR
-extern void * memchr(const void *, int, __kernel_size_t);
+void *memchr(const void *, int, size_t);
#if defined(CONFIG_ARM_32)
-extern void __memzero(void *ptr, __kernel_size_t n);
+void __memzero(void *ptr, size_t n);
-#define memset(p,v,n) \
+#define memset(p, v, n) \
({ \
void *__p = (p); size_t __n = n; \
if ((__n) != 0) { \
diff --git a/xen/include/xen/string.h b/xen/include/xen/string.h
index f35934e..eb7aeaa 100644
--- a/xen/include/xen/string.h
+++ b/xen/include/xen/string.h
@@ -1,19 +1,8 @@
-#ifndef _LINUX_STRING_H_
-#define _LINUX_STRING_H_
+#ifndef __XEN_STRING_H__
+#define __XEN_STRING_H__
#include <xen/types.h> /* for size_t */
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-#define __kernel_size_t size_t
-
-extern char * strpbrk(const char *,const char *);
-extern char * strsep(char **,const char *);
-extern __kernel_size_t strspn(const char *,const char *);
-
-
/*
* Include machine specific inline routines
*/
@@ -29,60 +18,84 @@ extern __kernel_size_t strspn(const char *,const char *);
#define strncat __xen_has_no_strncat__
#ifndef __HAVE_ARCH_STRLCPY
-extern size_t strlcpy(char *,const char *, __kernel_size_t);
+size_t strlcpy(char *, const char *, size_t);
#endif
+
#ifndef __HAVE_ARCH_STRLCAT
-extern size_t strlcat(char *,const char *, __kernel_size_t);
+size_t strlcat(char *, const char *, size_t);
#endif
+
#ifndef __HAVE_ARCH_STRCMP
-extern int strcmp(const char *,const char *);
+int strcmp(const char *, const char *);
#endif
+
#ifndef __HAVE_ARCH_STRNCMP
-extern int strncmp(const char *,const char *,__kernel_size_t);
+int strncmp(const char *, const char *, size_t);
#endif
+
#ifndef __HAVE_ARCH_STRNICMP
-extern int strnicmp(const char *, const char *, __kernel_size_t);
+int strnicmp(const char *, const char *, size_t);
#endif
+
#ifndef __HAVE_ARCH_STRCASECMP
-extern int strcasecmp(const char *, const char *);
+int strcasecmp(const char *, const char *);
#endif
+
#ifndef __HAVE_ARCH_STRCHR
-extern char * strchr(const char *,int);
+char *strchr(const char *, int);
#endif
+
#ifndef __HAVE_ARCH_STRRCHR
-extern char * strrchr(const char *,int);
+char *strrchr(const char *, int);
#endif
+
#ifndef __HAVE_ARCH_STRSTR
-extern char * strstr(const char *,const char *);
+char *strstr(const char *, const char *);
#endif
+
#ifndef __HAVE_ARCH_STRLEN
-extern __kernel_size_t strlen(const char *);
+size_t strlen(const char *);
#endif
+
#ifndef __HAVE_ARCH_STRNLEN
-extern __kernel_size_t strnlen(const char *,__kernel_size_t);
+size_t strnlen(const char *, size_t);
#endif
+#ifndef __HAVE_ARCH_STRPBRK
+char *strpbrk(const char *, const char *);
+#endif
+
+#ifndef __HAVE_ARCH_STRSEP
+char *strsep(char **, const char *);
+#endif
+
+#ifndef __HAVE_ARCH_STRSPN
+size_t strspn(const char *, const char *);
+#endif
+
+
#ifndef __HAVE_ARCH_MEMSET
-extern void * memset(void *,int,__kernel_size_t);
+void *memset(void *, int, size_t);
#endif
+
#ifndef __HAVE_ARCH_MEMCPY
-extern void * memcpy(void *,const void *,__kernel_size_t);
+void *memcpy(void *, const void *, size_t);
#endif
+
#ifndef __HAVE_ARCH_MEMMOVE
-extern void * memmove(void *,const void *,__kernel_size_t);
+void *memmove(void *, const void *, size_t);
#endif
+
#ifndef __HAVE_ARCH_MEMSCAN
-extern void * memscan(void *,int,__kernel_size_t);
+void *memscan(void *, int, size_t);
#endif
+
#ifndef __HAVE_ARCH_MEMCMP
-extern int memcmp(const void *,const void *,__kernel_size_t);
-#endif
-#ifndef __HAVE_ARCH_MEMCHR
-extern void * memchr(const void *,int,__kernel_size_t);
+int memcmp(const void *, const void *, size_t);
#endif
-#ifdef __cplusplus
-}
+#ifndef __HAVE_ARCH_MEMCHR
+void *memchr(const void *, int, size_t);
#endif
#define is_char_array(x) __builtin_types_compatible_p(typeof(x), char[])
@@ -97,4 +110,12 @@ extern void * memchr(const void *,int,__kernel_size_t);
(strlcat(d, s, sizeof(d)) >= sizeof(d)); \
})
-#endif /* _LINUX_STRING_H_ */
+#endif /* __XEN_STRING_H__ */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/3] xen/string: Clean up {xen, arm}/string.h
2017-05-12 17:35 ` [PATCH v3 1/3] xen/string: Clean up {xen, arm}/string.h Andrew Cooper
@ 2017-05-15 10:02 ` Jan Beulich
2017-05-31 8:47 ` Julien Grall
1 sibling, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2017-05-15 10:02 UTC (permalink / raw)
To: Andrew Cooper; +Cc: JulienGrall, Stefano Stabellini, Xen-devel
>>> On 12.05.17 at 19:35, <andrew.cooper3@citrix.com> wrote:
> * Drop __kernel_size_t entirely. It isn't useful distinction, especially as
> it means the the prototypes don't appear to match their common definitions.
> * Introduce __HAVE_ARCH_* guards for strpbrk(), strsep() and strspn(), which
> match their implementation in common/string.c
> * Apply consistent Xen style throughout.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/3] xen/string: Clean up {xen, arm}/string.h
2017-05-12 17:35 ` [PATCH v3 1/3] xen/string: Clean up {xen, arm}/string.h Andrew Cooper
2017-05-15 10:02 ` Jan Beulich
@ 2017-05-31 8:47 ` Julien Grall
1 sibling, 0 replies; 13+ messages in thread
From: Julien Grall @ 2017-05-31 8:47 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Stefano Stabellini, Jan Beulich
Hi Andrew,
On 05/12/2017 06:35 PM, Andrew Cooper wrote:
> * Drop __kernel_size_t entirely. It isn't useful distinction, especially as
> it means the the prototypes don't appear to match their common definitions.
> * Introduce __HAVE_ARCH_* guards for strpbrk(), strsep() and strspn(), which
> match their implementation in common/string.c
> * Apply consistent Xen style throughout.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
For ARM bits:
Acked-by: Julien Grall <julien.grall@arm.com>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 2/3] xen/string: Use compiler __builtin_*() where possible
2017-05-12 17:35 [PATCH v3 for-next 0/3] Improvements to string.h Andrew Cooper
2017-05-12 17:35 ` [PATCH v3 1/3] xen/string: Clean up {xen, arm}/string.h Andrew Cooper
@ 2017-05-12 17:35 ` Andrew Cooper
2017-05-15 10:05 ` Jan Beulich
2017-05-12 17:35 ` [PATCH v3 3/3] x86/string: Clean up x86/string.h Andrew Cooper
2 siblings, 1 reply; 13+ messages in thread
From: Andrew Cooper @ 2017-05-12 17:35 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Jan Beulich
The use of -fno-builtin inhibits these automatic transformations. This causes
constructs such as strlen("literal") to be evaluated at compile time, and
certain simple operations to be replaced with repeated string operations.
To avoid the macro altering the function names, use the method recommended by
the C specification by enclosing the function name in brackets to avoid the
macro being expanded. This means that optimisation opportunities continue to
work in the rest of the translation unit.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
v2:
* Fix the build with Clang, which objects when the define renames the
underlying implementation.
v3:
* Move into common code.
* Retain symbol definition for function pointer use.
---
xen/common/string.c | 24 ++++++++++++------------
xen/include/xen/string.h | 12 ++++++++++++
2 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/xen/common/string.c b/xen/common/string.c
index 9a5a4ba..1e122ab 100644
--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -42,7 +42,7 @@ int strnicmp(const char *s1, const char *s2, size_t len)
#endif
#ifndef __HAVE_ARCH_STRCASECMP
-int strcasecmp(const char *s1, const char *s2)
+int (strcasecmp)(const char *s1, const char *s2)
{
int c1, c2;
@@ -117,7 +117,7 @@ EXPORT_SYMBOL(strlcat);
* @cs: One string
* @ct: Another string
*/
-int strcmp(const char * cs,const char * ct)
+int (strcmp)(const char *cs, const char *ct)
{
register signed char __res;
@@ -137,7 +137,7 @@ int strcmp(const char * cs,const char * ct)
* @ct: Another string
* @count: The maximum number of bytes to compare
*/
-int strncmp(const char * cs,const char * ct,size_t count)
+int (strncmp)(const char *cs, const char *ct, size_t count)
{
register signed char __res = 0;
@@ -157,7 +157,7 @@ int strncmp(const char * cs,const char * ct,size_t count)
* @s: The string to be searched
* @c: The character to search for
*/
-char * strchr(const char * s, int c)
+char *(strchr)(const char *s, int c)
{
for(; *s != (char) c; ++s)
if (*s == '\0')
@@ -172,7 +172,7 @@ char * strchr(const char * s, int c)
* @s: The string to be searched
* @c: The character to search for
*/
-char * strrchr(const char * s, int c)
+char *(strrchr)(const char *s, int c)
{
const char *p = s + strlen(s);
do {
@@ -188,7 +188,7 @@ char * strrchr(const char * s, int c)
* strlen - Find the length of a string
* @s: The string to be sized
*/
-size_t strlen(const char * s)
+size_t (strlen)(const char * s)
{
const char *sc;
@@ -298,7 +298,7 @@ char * strsep(char **s, const char *ct)
*
* Do not use memset() to access IO space, use memset_io() instead.
*/
-void * memset(void * s,int c,size_t count)
+void *(memset)(void *s, int c, size_t count)
{
char *xs = (char *) s;
@@ -319,7 +319,7 @@ void * memset(void * s,int c,size_t count)
* You should not use this function to access IO space, use memcpy_toio()
* or memcpy_fromio() instead.
*/
-void * memcpy(void * dest,const void *src,size_t count)
+void *(memcpy)(void *dest, const void *src, size_t count)
{
char *tmp = (char *) dest, *s = (char *) src;
@@ -339,7 +339,7 @@ void * memcpy(void * dest,const void *src,size_t count)
*
* Unlike memcpy(), memmove() copes with overlapping areas.
*/
-void * memmove(void * dest,const void *src,size_t count)
+void *(memmove)(void *dest, const void *src, size_t count)
{
char *tmp, *s;
@@ -367,7 +367,7 @@ void * memmove(void * dest,const void *src,size_t count)
* @ct: Another area of memory
* @count: The size of the area.
*/
-int memcmp(const void * cs,const void * ct,size_t count)
+int (memcmp)(const void *cs, const void *ct, size_t count)
{
const unsigned char *su1, *su2;
int res = 0;
@@ -409,7 +409,7 @@ void * memscan(void * addr, int c, size_t size)
* @s1: The string to be searched
* @s2: The string to search for
*/
-char * strstr(const char * s1,const char * s2)
+char *(strstr)(const char *s1, const char *s2)
{
int l1, l2;
@@ -437,7 +437,7 @@ char * strstr(const char * s1,const char * s2)
* returns the address of the first occurrence of @c, or %NULL
* if @c is not found
*/
-void *memchr(const void *s, int c, size_t n)
+void *(memchr)(const void *s, int c, size_t n)
{
const unsigned char *p = s;
while (n-- != 0) {
diff --git a/xen/include/xen/string.h b/xen/include/xen/string.h
index eb7aeaa..853db2f 100644
--- a/xen/include/xen/string.h
+++ b/xen/include/xen/string.h
@@ -27,10 +27,12 @@ size_t strlcat(char *, const char *, size_t);
#ifndef __HAVE_ARCH_STRCMP
int strcmp(const char *, const char *);
+#define strcmp(s1, s2) __builtin_strcmp(s1, s2)
#endif
#ifndef __HAVE_ARCH_STRNCMP
int strncmp(const char *, const char *, size_t);
+#define strncmp(s1, s2, n) __builtin_strncmp(s1, s2, n)
#endif
#ifndef __HAVE_ARCH_STRNICMP
@@ -39,22 +41,27 @@ int strnicmp(const char *, const char *, size_t);
#ifndef __HAVE_ARCH_STRCASECMP
int strcasecmp(const char *, const char *);
+#define strcasecmp(s1, s2) __builtin_strcasecmp(s1, s2)
#endif
#ifndef __HAVE_ARCH_STRCHR
char *strchr(const char *, int);
+#define strchr(s1, c) __builtin_strchr(s1, c)
#endif
#ifndef __HAVE_ARCH_STRRCHR
char *strrchr(const char *, int);
+#define strrchr(s1, c) __builtin_strrchr(s1, c)
#endif
#ifndef __HAVE_ARCH_STRSTR
char *strstr(const char *, const char *);
+#define strstr(s1, s2) __builtin_strstr(s1, s2)
#endif
#ifndef __HAVE_ARCH_STRLEN
size_t strlen(const char *);
+#define strlen(s1) __builtin_strlen(s1)
#endif
#ifndef __HAVE_ARCH_STRNLEN
@@ -76,14 +83,17 @@ size_t strspn(const char *, const char *);
#ifndef __HAVE_ARCH_MEMSET
void *memset(void *, int, size_t);
+#define memset(s, c, n) __builtin_memset(s, c, n)
#endif
#ifndef __HAVE_ARCH_MEMCPY
void *memcpy(void *, const void *, size_t);
+#define memcpy(d, s, n) __builtin_memcpy(d, s, n)
#endif
#ifndef __HAVE_ARCH_MEMMOVE
void *memmove(void *, const void *, size_t);
+#define memmove(d, s, n) __builtin_memmove(d, s, n)
#endif
#ifndef __HAVE_ARCH_MEMSCAN
@@ -92,10 +102,12 @@ void *memscan(void *, int, size_t);
#ifndef __HAVE_ARCH_MEMCMP
int memcmp(const void *, const void *, size_t);
+#define memcmp(s1, s2, n) __builtin_memcmp(s1, s2, n)
#endif
#ifndef __HAVE_ARCH_MEMCHR
void *memchr(const void *, int, size_t);
+#define memchr(s, c, n) __builtin_memchr(s, c, n)
#endif
#define is_char_array(x) __builtin_types_compatible_p(typeof(x), char[])
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/3] xen/string: Use compiler __builtin_*() where possible
2017-05-12 17:35 ` [PATCH v3 2/3] xen/string: Use compiler __builtin_*() where possible Andrew Cooper
@ 2017-05-15 10:05 ` Jan Beulich
0 siblings, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2017-05-15 10:05 UTC (permalink / raw)
To: Andrew Cooper; +Cc: JulienGrall, Stefano Stabellini, Xen-devel
>>> On 12.05.17 at 19:35, <andrew.cooper3@citrix.com> wrote:
> The use of -fno-builtin inhibits these automatic transformations. This causes
> constructs such as strlen("literal") to be evaluated at compile time, and
> certain simple operations to be replaced with repeated string operations.
>
> To avoid the macro altering the function names, use the method recommended by
> the C specification by enclosing the function name in brackets to avoid the
> macro being expanded. This means that optimisation opportunities continue to
> work in the rest of the translation unit.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 3/3] x86/string: Clean up x86/string.h
2017-05-12 17:35 [PATCH v3 for-next 0/3] Improvements to string.h Andrew Cooper
2017-05-12 17:35 ` [PATCH v3 1/3] xen/string: Clean up {xen, arm}/string.h Andrew Cooper
2017-05-12 17:35 ` [PATCH v3 2/3] xen/string: Use compiler __builtin_*() where possible Andrew Cooper
@ 2017-05-12 17:35 ` Andrew Cooper
2017-05-15 10:08 ` Jan Beulich
2 siblings, 1 reply; 13+ messages in thread
From: Andrew Cooper @ 2017-05-12 17:35 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich
* None of the GCC docs mention memmove() in its list of builtins even today,
but 4.1 does have the builtin, meaning that all currently supported
compilers have it.
* Consistently use Xen style, matching the common code, and introduce symbol
definitions for function pointer use.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=21602 confirms that the builtin
was present in 4.1.0
v3:
* Rebase around the rework in the rest of the series
---
xen/arch/x86/string.c | 11 ++++-------
xen/include/asm-x86/string.h | 18 ++++++++++++++----
2 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/xen/arch/x86/string.c b/xen/arch/x86/string.c
index 1387dfb..cd85a38 100644
--- a/xen/arch/x86/string.c
+++ b/xen/arch/x86/string.c
@@ -1,14 +1,13 @@
/******************************************************************************
* string.c
- *
+ *
* These provide something for compiler-emitted string operations to link
* against.
*/
#include <xen/lib.h>
-#undef memcpy
-void *memcpy(void *dest, const void *src, size_t n)
+void *(memcpy)(void *dest, const void *src, size_t n)
{
long d0, d1, d2;
@@ -23,8 +22,7 @@ void *memcpy(void *dest, const void *src, size_t n)
return dest;
}
-#undef memset
-void *memset(void *s, int c, size_t n)
+void *(memset)(void *s, int c, size_t n)
{
long d0, d1;
@@ -37,8 +35,7 @@ void *memset(void *s, int c, size_t n)
return s;
}
-#undef memmove
-void *memmove(void *dest, const void *src, size_t n)
+void *(memmove)(void *dest, const void *src, size_t n)
{
long d0, d1, d2;
diff --git a/xen/include/asm-x86/string.h b/xen/include/asm-x86/string.h
index c48d9c3..6f3c960 100644
--- a/xen/include/asm-x86/string.h
+++ b/xen/include/asm-x86/string.h
@@ -2,13 +2,23 @@
#define __X86_STRING_H__
#define __HAVE_ARCH_MEMCPY
-#define memcpy(t,f,n) (__builtin_memcpy((t),(f),(n)))
+void *memcpy(void *dest, const void *src, size_t n);
+#define memcpy(d, s, n) __builtin_memcpy(d, s, n)
-/* Some versions of gcc don't have this builtin. It's non-critical anyway. */
#define __HAVE_ARCH_MEMMOVE
-extern void *memmove(void *dest, const void *src, size_t n);
+void *memmove(void *dest, const void *src, size_t n);
+#define memmove(d, s, n) __builtin_memmove(d, s, n)
#define __HAVE_ARCH_MEMSET
-#define memset(s,c,n) (__builtin_memset((s),(c),(n)))
+void *memset(void *dest, int c, size_t n);
+#define memset(s, c, n) __builtin_memset(s, c, n)
#endif /* __X86_STRING_H__ */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] x86/string: Clean up x86/string.h
2017-05-12 17:35 ` [PATCH v3 3/3] x86/string: Clean up x86/string.h Andrew Cooper
@ 2017-05-15 10:08 ` Jan Beulich
2017-05-15 10:19 ` Jan Beulich
0 siblings, 1 reply; 13+ messages in thread
From: Jan Beulich @ 2017-05-15 10:08 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 12.05.17 at 19:35, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/include/asm-x86/string.h
> +++ b/xen/include/asm-x86/string.h
> @@ -2,13 +2,23 @@
> #define __X86_STRING_H__
>
> #define __HAVE_ARCH_MEMCPY
> -#define memcpy(t,f,n) (__builtin_memcpy((t),(f),(n)))
> +void *memcpy(void *dest, const void *src, size_t n);
> +#define memcpy(d, s, n) __builtin_memcpy(d, s, n)
>
> -/* Some versions of gcc don't have this builtin. It's non-critical anyway. */
> #define __HAVE_ARCH_MEMMOVE
> -extern void *memmove(void *dest, const void *src, size_t n);
> +void *memmove(void *dest, const void *src, size_t n);
> +#define memmove(d, s, n) __builtin_memmove(d, s, n)
>
> #define __HAVE_ARCH_MEMSET
> -#define memset(s,c,n) (__builtin_memset((s),(c),(n)))
> +void *memset(void *dest, int c, size_t n);
> +#define memset(s, c, n) __builtin_memset(s, c, n)
Now that xen/string.h has the exact same declarations and
definitions already, why don't you simply delete the overrides
from here?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] x86/string: Clean up x86/string.h
2017-05-15 10:08 ` Jan Beulich
@ 2017-05-15 10:19 ` Jan Beulich
2017-05-15 13:08 ` Andrew Cooper
0 siblings, 1 reply; 13+ messages in thread
From: Jan Beulich @ 2017-05-15 10:19 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 15.05.17 at 12:08, <JBeulich@suse.com> wrote:
>>>> On 12.05.17 at 19:35, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/include/asm-x86/string.h
>> +++ b/xen/include/asm-x86/string.h
>> @@ -2,13 +2,23 @@
>> #define __X86_STRING_H__
>>
>> #define __HAVE_ARCH_MEMCPY
>> -#define memcpy(t,f,n) (__builtin_memcpy((t),(f),(n)))
>> +void *memcpy(void *dest, const void *src, size_t n);
>> +#define memcpy(d, s, n) __builtin_memcpy(d, s, n)
>>
>> -/* Some versions of gcc don't have this builtin. It's non-critical anyway.
> */
>> #define __HAVE_ARCH_MEMMOVE
>> -extern void *memmove(void *dest, const void *src, size_t n);
>> +void *memmove(void *dest, const void *src, size_t n);
>> +#define memmove(d, s, n) __builtin_memmove(d, s, n)
>>
>> #define __HAVE_ARCH_MEMSET
>> -#define memset(s,c,n) (__builtin_memset((s),(c),(n)))
>> +void *memset(void *dest, int c, size_t n);
>> +#define memset(s, c, n) __builtin_memset(s, c, n)
>
> Now that xen/string.h has the exact same declarations and
> definitions already, why don't you simply delete the overrides
> from here?
Hmm, wait - I guess you need to keep them because of the custom
implementation. That's awkward, there shouldn't be a need to have
redundant declarations just because there are custom
implementations. How about making __HAVE_ARCH_* serve both
purposes, by allowing it to have different values (besides being
defined or undefined)?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] x86/string: Clean up x86/string.h
2017-05-15 10:19 ` Jan Beulich
@ 2017-05-15 13:08 ` Andrew Cooper
2017-05-15 14:22 ` Jan Beulich
0 siblings, 1 reply; 13+ messages in thread
From: Andrew Cooper @ 2017-05-15 13:08 UTC (permalink / raw)
To: Jan Beulich; +Cc: Xen-devel
On 15/05/17 11:19, Jan Beulich wrote:
> >>> On 15.05.17 at 12:08, <JBeulich@suse.com> wrote:
>>>>> On 12.05.17 at 19:35, <andrew.cooper3@citrix.com> wrote:
>>> --- a/xen/include/asm-x86/string.h
>>> +++ b/xen/include/asm-x86/string.h
>>> @@ -2,13 +2,23 @@
>>> #define __X86_STRING_H__
>>>
>>> #define __HAVE_ARCH_MEMCPY
>>> -#define memcpy(t,f,n) (__builtin_memcpy((t),(f),(n)))
>>> +void *memcpy(void *dest, const void *src, size_t n);
>>> +#define memcpy(d, s, n) __builtin_memcpy(d, s, n)
>>>
>>> -/* Some versions of gcc don't have this builtin. It's non-critical anyway.
>> */
>>> #define __HAVE_ARCH_MEMMOVE
>>> -extern void *memmove(void *dest, const void *src, size_t n);
>>> +void *memmove(void *dest, const void *src, size_t n);
>>> +#define memmove(d, s, n) __builtin_memmove(d, s, n)
>>>
>>> #define __HAVE_ARCH_MEMSET
>>> -#define memset(s,c,n) (__builtin_memset((s),(c),(n)))
>>> +void *memset(void *dest, int c, size_t n);
>>> +#define memset(s, c, n) __builtin_memset(s, c, n)
>> Now that xen/string.h has the exact same declarations and
>> definitions already, why don't you simply delete the overrides
>> from here?
> Hmm, wait - I guess you need to keep them because of the custom
> implementation. That's awkward, there shouldn't be a need to have
> redundant declarations just because there are custom
> implementations. How about making __HAVE_ARCH_* serve both
> purposes, by allowing it to have different values (besides being
> defined or undefined)?
I don't understand how you would intend this new __HAVE_ARCH_* to work.
ARM uses the current behaviour to implement memset() as a local macro.
Given that these files aren't likely to change much at all, I'd
recommend keeping the same behaviour as Linux. It is only 3 lines after
all.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] x86/string: Clean up x86/string.h
2017-05-15 13:08 ` Andrew Cooper
@ 2017-05-15 14:22 ` Jan Beulich
2017-05-31 12:42 ` Andrew Cooper
0 siblings, 1 reply; 13+ messages in thread
From: Jan Beulich @ 2017-05-15 14:22 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 15.05.17 at 15:08, <andrew.cooper3@citrix.com> wrote:
> On 15/05/17 11:19, Jan Beulich wrote:
>> >>> On 15.05.17 at 12:08, <JBeulich@suse.com> wrote:
>>>>>> On 12.05.17 at 19:35, <andrew.cooper3@citrix.com> wrote:
>>>> --- a/xen/include/asm-x86/string.h
>>>> +++ b/xen/include/asm-x86/string.h
>>>> @@ -2,13 +2,23 @@
>>>> #define __X86_STRING_H__
>>>>
>>>> #define __HAVE_ARCH_MEMCPY
>>>> -#define memcpy(t,f,n) (__builtin_memcpy((t),(f),(n)))
>>>> +void *memcpy(void *dest, const void *src, size_t n);
>>>> +#define memcpy(d, s, n) __builtin_memcpy(d, s, n)
>>>>
>>>> -/* Some versions of gcc don't have this builtin. It's non-critical anyway.
>>> */
>>>> #define __HAVE_ARCH_MEMMOVE
>>>> -extern void *memmove(void *dest, const void *src, size_t n);
>>>> +void *memmove(void *dest, const void *src, size_t n);
>>>> +#define memmove(d, s, n) __builtin_memmove(d, s, n)
>>>>
>>>> #define __HAVE_ARCH_MEMSET
>>>> -#define memset(s,c,n) (__builtin_memset((s),(c),(n)))
>>>> +void *memset(void *dest, int c, size_t n);
>>>> +#define memset(s, c, n) __builtin_memset(s, c, n)
>>> Now that xen/string.h has the exact same declarations and
>>> definitions already, why don't you simply delete the overrides
>>> from here?
>> Hmm, wait - I guess you need to keep them because of the custom
>> implementation. That's awkward, there shouldn't be a need to have
>> redundant declarations just because there are custom
>> implementations. How about making __HAVE_ARCH_* serve both
>> purposes, by allowing it to have different values (besides being
>> defined or undefined)?
>
> I don't understand how you would intend this new __HAVE_ARCH_* to work.
E.g. __HAVE_ARCH_* = 2 meaning arch provides declaration and
definition (i.e. generic header and source skip theirs), while
__HAVE_ARCH_* = 1 meaning arch provides just a definition, but
the generic declaration and macro (where applicable) are fine (i.e.
only the generic source skips its piece of code).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] x86/string: Clean up x86/string.h
2017-05-15 14:22 ` Jan Beulich
@ 2017-05-31 12:42 ` Andrew Cooper
2017-05-31 13:00 ` Jan Beulich
0 siblings, 1 reply; 13+ messages in thread
From: Andrew Cooper @ 2017-05-31 12:42 UTC (permalink / raw)
To: Jan Beulich; +Cc: Xen-devel
On 15/05/17 15:22, Jan Beulich wrote:
>>>> On 15.05.17 at 15:08, <andrew.cooper3@citrix.com> wrote:
>> On 15/05/17 11:19, Jan Beulich wrote:
>>> >>> On 15.05.17 at 12:08, <JBeulich@suse.com> wrote:
>>>>>>> On 12.05.17 at 19:35, <andrew.cooper3@citrix.com> wrote:
>>>>> --- a/xen/include/asm-x86/string.h
>>>>> +++ b/xen/include/asm-x86/string.h
>>>>> @@ -2,13 +2,23 @@
>>>>> #define __X86_STRING_H__
>>>>>
>>>>> #define __HAVE_ARCH_MEMCPY
>>>>> -#define memcpy(t,f,n) (__builtin_memcpy((t),(f),(n)))
>>>>> +void *memcpy(void *dest, const void *src, size_t n);
>>>>> +#define memcpy(d, s, n) __builtin_memcpy(d, s, n)
>>>>>
>>>>> -/* Some versions of gcc don't have this builtin. It's non-critical anyway.
>>>> */
>>>>> #define __HAVE_ARCH_MEMMOVE
>>>>> -extern void *memmove(void *dest, const void *src, size_t n);
>>>>> +void *memmove(void *dest, const void *src, size_t n);
>>>>> +#define memmove(d, s, n) __builtin_memmove(d, s, n)
>>>>>
>>>>> #define __HAVE_ARCH_MEMSET
>>>>> -#define memset(s,c,n) (__builtin_memset((s),(c),(n)))
>>>>> +void *memset(void *dest, int c, size_t n);
>>>>> +#define memset(s, c, n) __builtin_memset(s, c, n)
>>>> Now that xen/string.h has the exact same declarations and
>>>> definitions already, why don't you simply delete the overrides
>>>> from here?
>>> Hmm, wait - I guess you need to keep them because of the custom
>>> implementation. That's awkward, there shouldn't be a need to have
>>> redundant declarations just because there are custom
>>> implementations. How about making __HAVE_ARCH_* serve both
>>> purposes, by allowing it to have different values (besides being
>>> defined or undefined)?
>> I don't understand how you would intend this new __HAVE_ARCH_* to work.
> E.g. __HAVE_ARCH_* = 2 meaning arch provides declaration and
> definition (i.e. generic header and source skip theirs), while
> __HAVE_ARCH_* = 1 meaning arch provides just a definition, but
> the generic declaration and macro (where applicable) are fine (i.e.
> only the generic source skips its piece of code).
I really don't thing its worth effort to make this change and diverge
from the Linux meaning.
It is not like these declarations are going to change often.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] x86/string: Clean up x86/string.h
2017-05-31 12:42 ` Andrew Cooper
@ 2017-05-31 13:00 ` Jan Beulich
0 siblings, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2017-05-31 13:00 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 31.05.17 at 14:42, <andrew.cooper3@citrix.com> wrote:
> On 15/05/17 15:22, Jan Beulich wrote:
>>>>> On 15.05.17 at 15:08, <andrew.cooper3@citrix.com> wrote:
>>> On 15/05/17 11:19, Jan Beulich wrote:
>>>> >>> On 15.05.17 at 12:08, <JBeulich@suse.com> wrote:
>>>>>>>> On 12.05.17 at 19:35, <andrew.cooper3@citrix.com> wrote:
>>>>>> --- a/xen/include/asm-x86/string.h
>>>>>> +++ b/xen/include/asm-x86/string.h
>>>>>> @@ -2,13 +2,23 @@
>>>>>> #define __X86_STRING_H__
>>>>>>
>>>>>> #define __HAVE_ARCH_MEMCPY
>>>>>> -#define memcpy(t,f,n) (__builtin_memcpy((t),(f),(n)))
>>>>>> +void *memcpy(void *dest, const void *src, size_t n);
>>>>>> +#define memcpy(d, s, n) __builtin_memcpy(d, s, n)
>>>>>>
>>>>>> -/* Some versions of gcc don't have this builtin. It's non-critical anyway.
>>>>> */
>>>>>> #define __HAVE_ARCH_MEMMOVE
>>>>>> -extern void *memmove(void *dest, const void *src, size_t n);
>>>>>> +void *memmove(void *dest, const void *src, size_t n);
>>>>>> +#define memmove(d, s, n) __builtin_memmove(d, s, n)
>>>>>>
>>>>>> #define __HAVE_ARCH_MEMSET
>>>>>> -#define memset(s,c,n) (__builtin_memset((s),(c),(n)))
>>>>>> +void *memset(void *dest, int c, size_t n);
>>>>>> +#define memset(s, c, n) __builtin_memset(s, c, n)
>>>>> Now that xen/string.h has the exact same declarations and
>>>>> definitions already, why don't you simply delete the overrides
>>>>> from here?
>>>> Hmm, wait - I guess you need to keep them because of the custom
>>>> implementation. That's awkward, there shouldn't be a need to have
>>>> redundant declarations just because there are custom
>>>> implementations. How about making __HAVE_ARCH_* serve both
>>>> purposes, by allowing it to have different values (besides being
>>>> defined or undefined)?
>>> I don't understand how you would intend this new __HAVE_ARCH_* to work.
>> E.g. __HAVE_ARCH_* = 2 meaning arch provides declaration and
>> definition (i.e. generic header and source skip theirs), while
>> __HAVE_ARCH_* = 1 meaning arch provides just a definition, but
>> the generic declaration and macro (where applicable) are fine (i.e.
>> only the generic source skips its piece of code).
>
> I really don't thing its worth effort to make this change and diverge
> from the Linux meaning.
>
> It is not like these declarations are going to change often.
Well, okay then. Feel free to add my ack.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread