[PATCH 00/23] further population of xen/lib/

[PATCH 00/23] further population of xen/lib/

[Jan Beulich]

This is to dissolve / move xen/common/lib.c and xen/common/string.c.
One benefit of moving these functions into an archive is that we can
drop some of the related __HAVE_ARCH_* #define-s: By living in an
archive, the per-arch functions will preempt any loading of the
respective functions (objects) from the archive. (Down the road we
may want to move the per-arch functions into archives as well, at
which point the per-arch archive(s) would need to be specified ahead
of the common one(s) to the linker.)

01: lib: move muldiv64()
02: lib: move 64-bit div/mod compiler helpers
03: string: drop redundant declarations
04: lib: move memset()
05: lib: move memcpy()
06: lib: move memmove()
07: lib: move memcmp()
08: lib: move memchr()
09: lib: move memchr_inv()
10: lib: move strlen()
11: lib: move strnlen()
12: lib: move strcmp()
13: lib: move strncmp()
14: lib: move strlcpy()
15: lib: move strlcat()
16: lib: move strchr()
17: lib: move strrchr()
18: lib: move strstr()
19: lib: move strcasecmp()
20: lib: move/rename strnicmp() to strncasecmp()
21: lib: move strspn()
22: lib: move strpbrk()
23: lib: move strsep()

Jan

[PATCH 01/23] lib: move muldiv64()

[Jan Beulich]

Make this a separate archive member under lib/. While doing so, don't
move latently broken x86 assembly though: Fix the constraints, such
that properly extending inputs to 64-bit won't just be a side effect of
needing to copy registers, and such that we won't fail to clobber %rdx.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/lib.c
+++ b/xen/common/lib.c
@@ -393,35 +393,6 @@ s64 __ldivmod_helper(s64 a, s64 b, s64 *
 }
 #endif /* BITS_PER_LONG == 32 */
 
-/* Compute with 96 bit intermediate result: (a*b)/c */
-uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
-{
-#ifdef CONFIG_X86
-    asm ( "mul %%rdx; div %%rcx" : "=a" (a) : "0" (a), "d" (b), "c" (c) );
-    return a;
-#else
-    union {
-        uint64_t ll;
-        struct {
-#ifdef WORDS_BIGENDIAN
-            uint32_t high, low;
-#else
-            uint32_t low, high;
-#endif            
-        } l;
-    } u, res;
-    uint64_t rl, rh;
-
-    u.ll = a;
-    rl = (uint64_t)u.l.low * (uint64_t)b;
-    rh = (uint64_t)u.l.high * (uint64_t)b;
-    rh += (rl >> 32);
-    res.l.high = rh / c;
-    res.l.low = (((rh % c) << 32) + (rl & 0xffffffff)) / c;
-    return res.ll;
-#endif
-}
-
 /*
  * Local variables:
  * mode: C
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -4,6 +4,7 @@ lib-y += bsearch.o
 lib-y += ctors.o
 lib-y += ctype.o
 lib-y += list-sort.o
+lib-y += muldiv64.o
 lib-y += parse-size.o
 lib-y += rbtree.o
 lib-y += sort.o
--- /dev/null
+++ b/xen/lib/muldiv64.c
@@ -0,0 +1,44 @@
+#include <xen/lib.h>
+
+/* Compute with 96 bit intermediate result: (a*b)/c */
+uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
+{
+#ifdef CONFIG_X86
+    asm ( "mulq %1; divq %2" : "+a" (a)
+                             : "rm" ((uint64_t)b), "rm" ((uint64_t)c)
+                             : "rdx" );
+
+    return a;
+#else
+    union {
+        uint64_t ll;
+        struct {
+#ifdef WORDS_BIGENDIAN
+            uint32_t high, low;
+#else
+            uint32_t low, high;
+#endif
+        } l;
+    } u, res;
+    uint64_t rl, rh;
+
+    u.ll = a;
+    rl = (uint64_t)u.l.low * (uint64_t)b;
+    rh = (uint64_t)u.l.high * (uint64_t)b;
+    rh += (rl >> 32);
+    res.l.high = rh / c;
+    res.l.low = (((rh % c) << 32) + (rl & 0xffffffff)) / c;
+
+    return res.ll;
+#endif
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */

Ping (x86): [PATCH 01/23] lib: move muldiv64()

[Jan Beulich]

On 01.04.2021 12:19, Jan Beulich wrote:
> Make this a separate archive member under lib/. While doing so, don't
> move latently broken x86 assembly though: Fix the constraints, such
> that properly extending inputs to 64-bit won't just be a side effect of
> needing to copy registers, and such that we won't fail to clobber %rdx.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

While this is a change to common code, there being a specific x86
aspect here I can understand that Julien would prefer to not ack
this without an x86 maintainer having reviewed the change. May I
ask for feedback here, as I'd prefer to commit at least the first
few patches of this series before re-posting the rest.

Thanks, Jan

> --- a/xen/common/lib.c
> +++ b/xen/common/lib.c
> @@ -393,35 +393,6 @@ s64 __ldivmod_helper(s64 a, s64 b, s64 *
>  }
>  #endif /* BITS_PER_LONG == 32 */
>  
> -/* Compute with 96 bit intermediate result: (a*b)/c */
> -uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
> -{
> -#ifdef CONFIG_X86
> -    asm ( "mul %%rdx; div %%rcx" : "=a" (a) : "0" (a), "d" (b), "c" (c) );
> -    return a;
> -#else
> -    union {
> -        uint64_t ll;
> -        struct {
> -#ifdef WORDS_BIGENDIAN
> -            uint32_t high, low;
> -#else
> -            uint32_t low, high;
> -#endif            
> -        } l;
> -    } u, res;
> -    uint64_t rl, rh;
> -
> -    u.ll = a;
> -    rl = (uint64_t)u.l.low * (uint64_t)b;
> -    rh = (uint64_t)u.l.high * (uint64_t)b;
> -    rh += (rl >> 32);
> -    res.l.high = rh / c;
> -    res.l.low = (((rh % c) << 32) + (rl & 0xffffffff)) / c;
> -    return res.ll;
> -#endif
> -}
> -
>  /*
>   * Local variables:
>   * mode: C
> --- a/xen/lib/Makefile
> +++ b/xen/lib/Makefile
> @@ -4,6 +4,7 @@ lib-y += bsearch.o
>  lib-y += ctors.o
>  lib-y += ctype.o
>  lib-y += list-sort.o
> +lib-y += muldiv64.o
>  lib-y += parse-size.o
>  lib-y += rbtree.o
>  lib-y += sort.o
> --- /dev/null
> +++ b/xen/lib/muldiv64.c
> @@ -0,0 +1,44 @@
> +#include <xen/lib.h>
> +
> +/* Compute with 96 bit intermediate result: (a*b)/c */
> +uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
> +{
> +#ifdef CONFIG_X86
> +    asm ( "mulq %1; divq %2" : "+a" (a)
> +                             : "rm" ((uint64_t)b), "rm" ((uint64_t)c)
> +                             : "rdx" );
> +
> +    return a;
> +#else
> +    union {
> +        uint64_t ll;
> +        struct {
> +#ifdef WORDS_BIGENDIAN
> +            uint32_t high, low;
> +#else
> +            uint32_t low, high;
> +#endif
> +        } l;
> +    } u, res;
> +    uint64_t rl, rh;
> +
> +    u.ll = a;
> +    rl = (uint64_t)u.l.low * (uint64_t)b;
> +    rh = (uint64_t)u.l.high * (uint64_t)b;
> +    rh += (rl >> 32);
> +    res.l.high = rh / c;
> +    res.l.low = (((rh % c) << 32) + (rl & 0xffffffff)) / c;
> +
> +    return res.ll;
> +#endif
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> 
> 

Re: [PATCH 01/23] lib: move muldiv64()

[Roger Pau Monné]

On Thu, Apr 01, 2021 at 12:19:16PM +0200, Jan Beulich wrote:
> Make this a separate archive member under lib/. While doing so, don't
> move latently broken x86 assembly though: Fix the constraints, such
> that properly extending inputs to 64-bit won't just be a side effect of
> needing to copy registers, and such that we won't fail to clobber %rdx.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

The x86 part LGTM:

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

Thanks, Roger.

[PATCH 02/23] lib: move 64-bit div/mod compiler helpers

[Jan Beulich]

These were built for 32-bit architectures only (the same code could,
with some tweaking, sensibly be used to provide TI-mode helpers on
64-bit arch-es) - retain this property, while still avoiding to have
a CU without any contents at all. For this, Arm's CONFIG_64BIT gets
generalized.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/Kconfig                   |  2 ++
 xen/arch/arm/Kconfig               | 12 +++---------
 xen/arch/x86/Kconfig               |  1 +
 xen/common/Makefile                |  1 -
 xen/lib/Makefile                   |  4 ++++
 xen/{common/lib.c => lib/divmod.c} |  2 --
 6 files changed, 10 insertions(+), 12 deletions(-)
 rename xen/{common/lib.c => lib/divmod.c} (99%)

diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig
index d144d4c8d3ee..f16eb0df43af 100644
--- a/xen/arch/Kconfig
+++ b/xen/arch/Kconfig
@@ -1,3 +1,5 @@
+config 64BIT
+	bool
 
 config NR_CPUS
 	int "Maximum number of CPUs"
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 330bbf6232d4..ecfa6822e4d3 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -1,17 +1,11 @@
-config 64BIT
-	bool
-	default "$(ARCH)" != "arm32"
-	help
-	  Say yes to build a 64-bit Xen
-	  Say no to build a 32-bit Xen
-
 config ARM_32
 	def_bool y
-	depends on !64BIT
+	depends on "$(ARCH)" = "arm32"
 
 config ARM_64
 	def_bool y
-	depends on 64BIT
+	depends on !ARM_32
+	select 64BIT
 	select HAS_FAST_MULTIPLY
 
 config ARM
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index f79e6634db3f..4d6911ffa467 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -1,5 +1,6 @@
 config X86_64
 	def_bool y
+	select 64BIT
 
 config X86
 	def_bool y
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 71c1d466bd8f..e2a7e62d14bf 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -21,7 +21,6 @@ obj-y += kernel.o
 obj-y += keyhandler.o
 obj-$(CONFIG_KEXEC) += kexec.o
 obj-$(CONFIG_KEXEC) += kimage.o
-obj-y += lib.o
 obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o
 obj-$(CONFIG_MEM_ACCESS) += mem_access.o
 obj-y += memory.o
diff --git a/xen/lib/Makefile b/xen/lib/Makefile
index 0b274583ef0b..a5dc1442a422 100644
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -10,3 +10,7 @@ lib-y += rbtree.o
 lib-y += sort.o
 lib-$(CONFIG_X86) += xxhash32.o
 lib-$(CONFIG_X86) += xxhash64.o
+
+lib32-y := divmod.o
+lib32-$(CONFIG_64BIT) :=
+lib-y += $(lib32-y)
diff --git a/xen/common/lib.c b/xen/lib/divmod.c
similarity index 99%
rename from xen/common/lib.c
rename to xen/lib/divmod.c
index 5b8f49153dad..0be6ccc70096 100644
--- a/xen/common/lib.c
+++ b/xen/lib/divmod.c
@@ -40,7 +40,6 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  */
-#if BITS_PER_LONG == 32
 
 /*
  * Depending on the desired operation, we view a `long long' (aka quad_t) in
@@ -391,7 +390,6 @@ s64 __ldivmod_helper(s64 a, s64 b, s64 *r)
     else
         return quot;
 }
-#endif /* BITS_PER_LONG == 32 */
 
 /*
  * Local variables:

Re: [PATCH 02/23] lib: move 64-bit div/mod compiler helpers

[Julien Grall]

Hi Jan,

On 01/04/2021 11:19, Jan Beulich wrote:
> These were built for 32-bit architectures only (the same code could,
> with some tweaking, sensibly be used to provide TI-mode helpers on
> 64-bit arch-es) - retain this property, while still avoiding to have
> a CU without any contents at all. For this, Arm's CONFIG_64BIT gets
> generalized.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
>   xen/arch/Kconfig                   |  2 ++
>   xen/arch/arm/Kconfig               | 12 +++---------
>   xen/arch/x86/Kconfig               |  1 +
>   xen/common/Makefile                |  1 -
>   xen/lib/Makefile                   |  4 ++++
>   xen/{common/lib.c => lib/divmod.c} |  2 --
>   6 files changed, 10 insertions(+), 12 deletions(-)
>   rename xen/{common/lib.c => lib/divmod.c} (99%)
> 
> diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig
> index d144d4c8d3ee..f16eb0df43af 100644
> --- a/xen/arch/Kconfig
> +++ b/xen/arch/Kconfig
> @@ -1,3 +1,5 @@
> +config 64BIT
> +	bool
>   
>   config NR_CPUS
>   	int "Maximum number of CPUs"
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 330bbf6232d4..ecfa6822e4d3 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -1,17 +1,11 @@
> -config 64BIT
> -	bool
> -	default "$(ARCH)" != "arm32"
> -	help
> -	  Say yes to build a 64-bit Xen
> -	  Say no to build a 32-bit Xen
> -
>   config ARM_32
>   	def_bool y
> -	depends on !64BIT
> +	depends on "$(ARCH)" = "arm32"
>   
>   config ARM_64
>   	def_bool y
> -	depends on 64BIT
> +	depends on !ARM_32
> +	select 64BIT
>   	select HAS_FAST_MULTIPLY
>   
>   config ARM
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index f79e6634db3f..4d6911ffa467 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -1,5 +1,6 @@
>   config X86_64
>   	def_bool y
> +	select 64BIT
>   
>   config X86
>   	def_bool y
> diff --git a/xen/common/Makefile b/xen/common/Makefile
> index 71c1d466bd8f..e2a7e62d14bf 100644
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -21,7 +21,6 @@ obj-y += kernel.o
>   obj-y += keyhandler.o
>   obj-$(CONFIG_KEXEC) += kexec.o
>   obj-$(CONFIG_KEXEC) += kimage.o
> -obj-y += lib.o
>   obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o
>   obj-$(CONFIG_MEM_ACCESS) += mem_access.o
>   obj-y += memory.o
> diff --git a/xen/lib/Makefile b/xen/lib/Makefile
> index 0b274583ef0b..a5dc1442a422 100644
> --- a/xen/lib/Makefile
> +++ b/xen/lib/Makefile
> @@ -10,3 +10,7 @@ lib-y += rbtree.o
>   lib-y += sort.o
>   lib-$(CONFIG_X86) += xxhash32.o
>   lib-$(CONFIG_X86) += xxhash64.o
> +
> +lib32-y := divmod.o
> +lib32-$(CONFIG_64BIT) :=
> +lib-y += $(lib32-y)
> diff --git a/xen/common/lib.c b/xen/lib/divmod.c
> similarity index 99%
> rename from xen/common/lib.c
> rename to xen/lib/divmod.c
> index 5b8f49153dad..0be6ccc70096 100644
> --- a/xen/common/lib.c
> +++ b/xen/lib/divmod.c
> @@ -40,7 +40,6 @@
>    * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>    * SUCH DAMAGE.
>    */
> -#if BITS_PER_LONG == 32

In theory BITS_PER_LONG == 32 is not quite the same as 32-bit arch. Can 
you clarify whether this code is necessary on 64-bit arch where long is 
only 32-bit.

Cheers?

-- 
Julien Grall

Re: [PATCH 02/23] lib: move 64-bit div/mod compiler helpers

[Jan Beulich]

On 01.04.2021 16:56, Julien Grall wrote:
> On 01/04/2021 11:19, Jan Beulich wrote:
>> --- a/xen/common/lib.c
>> +++ b/xen/lib/divmod.c
>> @@ -40,7 +40,6 @@
>>    * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>>    * SUCH DAMAGE.
>>    */
>> -#if BITS_PER_LONG == 32
> 
> In theory BITS_PER_LONG == 32 is not quite the same as 32-bit arch. Can 
> you clarify whether this code is necessary on 64-bit arch where long is 
> only 32-bit.

Likely the compiler can get away without invoking these helpers, so
the code would remain unused. I'm uncertain whether CONFIG_64BIT
ought to be set for such an architecture, as we assume sizeof(long)
== sizeof(void*), and hence pointers would then need to be 32-bit
as well there.

Jan

Re: [PATCH 02/23] lib: move 64-bit div/mod compiler helpers

[Julien Grall]

Hi Jan,

On 01/04/2021 16:23, Jan Beulich wrote:
> On 01.04.2021 16:56, Julien Grall wrote:
>> On 01/04/2021 11:19, Jan Beulich wrote:
>>> --- a/xen/common/lib.c
>>> +++ b/xen/lib/divmod.c
>>> @@ -40,7 +40,6 @@
>>>     * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>>>     * SUCH DAMAGE.
>>>     */
>>> -#if BITS_PER_LONG == 32
>>
>> In theory BITS_PER_LONG == 32 is not quite the same as 32-bit arch. Can
>> you clarify whether this code is necessary on 64-bit arch where long is
>> only 32-bit.
> 
> Likely the compiler can get away without invoking these helpers, so
> the code would remain unused. I'm uncertain whether CONFIG_64BIT
> ought to be set for such an architecture, as we assume sizeof(long)
> == sizeof(void*), and hence pointers would then need to be 32-bit
> as well there.

This is a fair point. Would you mind to add a sentence explaining that 
in the commit message?

With that:

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

Re: [PATCH 02/23] lib: move 64-bit div/mod compiler helpers

[Jan Beulich]

On 06.04.2021 21:34, Julien Grall wrote:
> Hi Jan,
> 
> On 01/04/2021 16:23, Jan Beulich wrote:
>> On 01.04.2021 16:56, Julien Grall wrote:
>>> On 01/04/2021 11:19, Jan Beulich wrote:
>>>> --- a/xen/common/lib.c
>>>> +++ b/xen/lib/divmod.c
>>>> @@ -40,7 +40,6 @@
>>>>     * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>>>>     * SUCH DAMAGE.
>>>>     */
>>>> -#if BITS_PER_LONG == 32
>>>
>>> In theory BITS_PER_LONG == 32 is not quite the same as 32-bit arch. Can
>>> you clarify whether this code is necessary on 64-bit arch where long is
>>> only 32-bit.
>>
>> Likely the compiler can get away without invoking these helpers, so
>> the code would remain unused. I'm uncertain whether CONFIG_64BIT
>> ought to be set for such an architecture, as we assume sizeof(long)
>> == sizeof(void*), and hence pointers would then need to be 32-bit
>> as well there.
> 
> This is a fair point. Would you mind to add a sentence explaining that 
> in the commit message?

I've added

"Note that we imply "32-bit arch" to be the same as BITS_PER_LONG == 32,
 i.e. we aren't (not just here) prepared to have a 64-bit arch with
 BITS_PER_LONG == 32. Yet even if we supported such, likely the compiler
 would get away there without invoking these helpers, so the code would
 remain unused in practice."

> With that:
> 
> Acked-by: Julien Grall <jgrall@amazon.com>

Thanks. Any chance to also get an ack on patch 1, so at least these two
(or three, seeing that you also did ack patch 3) could go in before my
re-posting of the series to add the one line commit message additions
that you did ask for on all the str* and mem* patches? (Alternatively I
could take the time and re-order the two.)

Jan

Re: [PATCH 02/23] lib: move 64-bit div/mod compiler helpers

[Julien Grall]

On 07/04/2021 09:33, Jan Beulich wrote:
> On 06.04.2021 21:34, Julien Grall wrote:
>> Hi Jan,
>>
>> On 01/04/2021 16:23, Jan Beulich wrote:
>>> On 01.04.2021 16:56, Julien Grall wrote:
>>>> On 01/04/2021 11:19, Jan Beulich wrote:
>>>>> --- a/xen/common/lib.c
>>>>> +++ b/xen/lib/divmod.c
>>>>> @@ -40,7 +40,6 @@
>>>>>      * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>>>>>      * SUCH DAMAGE.
>>>>>      */
>>>>> -#if BITS_PER_LONG == 32
>>>>
>>>> In theory BITS_PER_LONG == 32 is not quite the same as 32-bit arch. Can
>>>> you clarify whether this code is necessary on 64-bit arch where long is
>>>> only 32-bit.
>>>
>>> Likely the compiler can get away without invoking these helpers, so
>>> the code would remain unused. I'm uncertain whether CONFIG_64BIT
>>> ought to be set for such an architecture, as we assume sizeof(long)
>>> == sizeof(void*), and hence pointers would then need to be 32-bit
>>> as well there.
>>
>> This is a fair point. Would you mind to add a sentence explaining that
>> in the commit message?
> 
> I've added
> 
> "Note that we imply "32-bit arch" to be the same as BITS_PER_LONG == 32,
>   i.e. we aren't (not just here) prepared to have a 64-bit arch with
>   BITS_PER_LONG == 32. Yet even if we supported such, likely the compiler
>   would get away there without invoking these helpers, so the code would
>   remain unused in practice."

Sounds fine to me.

> 
>> With that:
>>
>> Acked-by: Julien Grall <jgrall@amazon.com>
> 
> Thanks. Any chance to also get an ack on patch 1, so at least these two
> (or three, seeing that you also did ack patch 3) could go in before my
> re-posting of the series to add the one line commit message additions
> that you did ask for on all the str* and mem* patches? (Alternatively I
> could take the time and re-order the two.)

I didn't ack #1 because I am not very familiar with the x86 constraints.
If anyone with x86 background (maybe Roger?) is willing to review it, 
then I would be happy to give my ack.

Cheers,

-- 
Julien Grall

[PATCH 03/23] string: drop redundant declarations

[Jan Beulich]

These standard functions shouldn't need custom declarations. The only
case where redundancy might be needed is if there were inline functions
there. But we don't have any here (anymore). Prune the per-arch headers
of duplicate declarations while moving the asm/string.h inclusion past
the declarations.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/include/asm-arm/string.h
+++ b/xen/include/asm-arm/string.h
@@ -8,41 +8,21 @@
  */
 
 #define __HAVE_ARCH_STRRCHR
-char *strrchr(const char *s, int c);
-
 #define __HAVE_ARCH_STRCHR
-char *strchr(const char *s, int c);
-
 #if defined(CONFIG_ARM_64)
 #define __HAVE_ARCH_STRCMP
-int strcmp(const char *, const char *);
-
 #define __HAVE_ARCH_STRNCMP
-int strncmp(const char *, const char *, size_t);
-
 #define __HAVE_ARCH_STRLEN
-size_t strlen(const char *);
-
 #define __HAVE_ARCH_STRNLEN
-size_t strnlen(const char *, size_t);
 #endif
 
 #define __HAVE_ARCH_MEMCPY
-void *memcpy(void *, const void *, size_t);
-
 #if defined(CONFIG_ARM_64)
 #define __HAVE_ARCH_MEMCMP
-int memcmp(const void *, const void *, size_t);
 #endif
-
 #define __HAVE_ARCH_MEMMOVE
-void *memmove(void *dest, const void *src, size_t n);
-
 #define __HAVE_ARCH_MEMSET
-void *memset(void *, int, size_t);
-
 #define __HAVE_ARCH_MEMCHR
-void *memchr(const void *, int, size_t);
 
 #if defined(CONFIG_ARM_32)
 
--- a/xen/include/asm-x86/string.h
+++ b/xen/include/asm-x86/string.h
@@ -2,15 +2,12 @@
 #define __X86_STRING_H__
 
 #define __HAVE_ARCH_MEMCPY
-void *memcpy(void *dest, const void *src, size_t n);
 #define memcpy(d, s, n) __builtin_memcpy(d, s, n)
 
 #define __HAVE_ARCH_MEMMOVE
-void *memmove(void *dest, const void *src, size_t n);
 #define memmove(d, s, n) __builtin_memmove(d, s, n)
 
 #define __HAVE_ARCH_MEMSET
-void *memset(void *dest, int c, size_t n);
 #define memset(s, c, n) __builtin_memset(s, c, n)
 
 #endif /* __X86_STRING_H__ */
--- a/xen/include/xen/string.h
+++ b/xen/include/xen/string.h
@@ -4,11 +4,6 @@
 #include <xen/types.h>	/* for size_t */
 
 /*
- * Include machine specific inline routines
- */
-#include <asm/string.h>
-
-/*
  * These string functions are considered too dangerous for normal use.
  * Use safe_strcpy(), safe_strcat(), strlcpy(), strlcat() as appropriate.
  */
@@ -17,97 +12,78 @@
 #define strncpy __xen_has_no_strncpy__
 #define strncat __xen_has_no_strncat__
 
-#ifndef __HAVE_ARCH_STRLCPY
 size_t strlcpy(char *, const char *, size_t);
-#endif
-
-#ifndef __HAVE_ARCH_STRLCAT
 size_t strlcat(char *, const char *, size_t);
-#endif
+int strcmp(const char *, const char *);
+int strncmp(const char *, const char *, size_t);
+int strnicmp(const char *, const char *, size_t);
+int strcasecmp(const char *, const char *);
+char *strchr(const char *, int);
+char *strrchr(const char *, int);
+char *strstr(const char *, const char *);
+size_t strlen(const char *);
+size_t strnlen(const char *, size_t);
+char *strpbrk(const char *, const char *);
+char *strsep(char **, const char *);
+size_t strspn(const char *, const char *);
+
+void *memset(void *, int, size_t);
+void *memcpy(void *, const void *, size_t);
+void *memmove(void *, const void *, size_t);
+int memcmp(const void *, const void *, size_t);
+void *memchr(const void *, int, size_t);
+void *memchr_inv(const void *, int, size_t);
+
+#include <asm/string.h>
 
 #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
-int strnicmp(const char *, const char *, size_t);
-#endif
-
 #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
-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
-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_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
 
-void *memchr_inv(const void *, int, size_t);
-
 #define is_char_array(x) __builtin_types_compatible_p(typeof(x), char[])
 
 /* safe_xxx always NUL-terminates and returns !=0 if result is truncated. */

Re: [PATCH 03/23] string: drop redundant declarations

[Julien Grall]

Hi Jan,

On 01/04/2021 11:20, Jan Beulich wrote:
> These standard functions shouldn't need custom declarations. The only
> case where redundancy might be needed is if there were inline functions
> there. But we don't have any here (anymore). Prune the per-arch headers
> of duplicate declarations while moving the asm/string.h inclusion past
> the declarations.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

> 
> --- a/xen/include/asm-arm/string.h
> +++ b/xen/include/asm-arm/string.h
> @@ -8,41 +8,21 @@
>    */
>   
>   #define __HAVE_ARCH_STRRCHR
> -char *strrchr(const char *s, int c);
> -
>   #define __HAVE_ARCH_STRCHR
> -char *strchr(const char *s, int c);
> -
>   #if defined(CONFIG_ARM_64)
>   #define __HAVE_ARCH_STRCMP
> -int strcmp(const char *, const char *);
> -
>   #define __HAVE_ARCH_STRNCMP
> -int strncmp(const char *, const char *, size_t);
> -
>   #define __HAVE_ARCH_STRLEN
> -size_t strlen(const char *);
> -
>   #define __HAVE_ARCH_STRNLEN
> -size_t strnlen(const char *, size_t);
>   #endif
>   
>   #define __HAVE_ARCH_MEMCPY
> -void *memcpy(void *, const void *, size_t);
> -
>   #if defined(CONFIG_ARM_64)
>   #define __HAVE_ARCH_MEMCMP
> -int memcmp(const void *, const void *, size_t);
>   #endif
> -
>   #define __HAVE_ARCH_MEMMOVE
> -void *memmove(void *dest, const void *src, size_t n);
> -
>   #define __HAVE_ARCH_MEMSET
> -void *memset(void *, int, size_t);
> -
>   #define __HAVE_ARCH_MEMCHR
> -void *memchr(const void *, int, size_t);
>   
>   #if defined(CONFIG_ARM_32)
>   
> --- a/xen/include/asm-x86/string.h
> +++ b/xen/include/asm-x86/string.h
> @@ -2,15 +2,12 @@
>   #define __X86_STRING_H__
>   
>   #define __HAVE_ARCH_MEMCPY
> -void *memcpy(void *dest, const void *src, size_t n);
>   #define memcpy(d, s, n) __builtin_memcpy(d, s, n)
>   
>   #define __HAVE_ARCH_MEMMOVE
> -void *memmove(void *dest, const void *src, size_t n);
>   #define memmove(d, s, n) __builtin_memmove(d, s, n)
>   
>   #define __HAVE_ARCH_MEMSET
> -void *memset(void *dest, int c, size_t n);
>   #define memset(s, c, n) __builtin_memset(s, c, n)
>   
>   #endif /* __X86_STRING_H__ */
> --- a/xen/include/xen/string.h
> +++ b/xen/include/xen/string.h
> @@ -4,11 +4,6 @@
>   #include <xen/types.h>	/* for size_t */
>   
>   /*
> - * Include machine specific inline routines
> - */
> -#include <asm/string.h>
> -
> -/*
>    * These string functions are considered too dangerous for normal use.
>    * Use safe_strcpy(), safe_strcat(), strlcpy(), strlcat() as appropriate.
>    */
> @@ -17,97 +12,78 @@
>   #define strncpy __xen_has_no_strncpy__
>   #define strncat __xen_has_no_strncat__
>   
> -#ifndef __HAVE_ARCH_STRLCPY
>   size_t strlcpy(char *, const char *, size_t);
> -#endif
> -
> -#ifndef __HAVE_ARCH_STRLCAT
>   size_t strlcat(char *, const char *, size_t);
> -#endif
> +int strcmp(const char *, const char *);
> +int strncmp(const char *, const char *, size_t);
> +int strnicmp(const char *, const char *, size_t);
> +int strcasecmp(const char *, const char *);
> +char *strchr(const char *, int);
> +char *strrchr(const char *, int);
> +char *strstr(const char *, const char *);
> +size_t strlen(const char *);
> +size_t strnlen(const char *, size_t);
> +char *strpbrk(const char *, const char *);
> +char *strsep(char **, const char *);
> +size_t strspn(const char *, const char *);
> +
> +void *memset(void *, int, size_t);
> +void *memcpy(void *, const void *, size_t);
> +void *memmove(void *, const void *, size_t);
> +int memcmp(const void *, const void *, size_t);
> +void *memchr(const void *, int, size_t);
> +void *memchr_inv(const void *, int, size_t);
> +
> +#include <asm/string.h>
>   
>   #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
> -int strnicmp(const char *, const char *, size_t);
> -#endif
> -
>   #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
> -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
> -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_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
>   
> -void *memchr_inv(const void *, int, size_t);
> -
>   #define is_char_array(x) __builtin_types_compatible_p(typeof(x), char[])
>   
>   /* safe_xxx always NUL-terminates and returns !=0 if result is truncated. */
> 

-- 
Julien Grall

[PATCH 04/23] lib: move memset()

[Jan Beulich]

By moving the function into an archive, x86 doesn't need to announce
anymore that is has its own implementation - symbol resolution by the
linker will now guarantee that the generic function remains unused, and
the forwarding to the compiler built-in gets done by the common header
anyway.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -311,26 +311,6 @@ char *(strstr)(const char *s1, const cha
 }
 #endif
 
-#ifndef __HAVE_ARCH_MEMSET
-/**
- * memset - Fill a region of memory with the given value
- * @s: Pointer to the start of the area.
- * @c: The byte to fill the area with
- * @count: The size of the area.
- *
- * Do not use memset() to access IO space, use memset_io() instead.
- */
-void *(memset)(void *s, int c, size_t count)
-{
-	char *xs = (char *) s;
-
-	while (count--)
-		*xs++ = c;
-
-	return s;
-}
-#endif
-
 #ifndef __HAVE_ARCH_MEMCPY
 /**
  * memcpy - Copy one area of memory to another
--- a/xen/include/asm-x86/string.h
+++ b/xen/include/asm-x86/string.h
@@ -7,9 +7,6 @@
 #define __HAVE_ARCH_MEMMOVE
 #define memmove(d, s, n) __builtin_memmove(d, s, n)
 
-#define __HAVE_ARCH_MEMSET
-#define memset(s, c, n) __builtin_memset(s, c, n)
-
 #endif /* __X86_STRING_H__ */
 /*
  * Local variables:
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -4,6 +4,7 @@ lib-y += bsearch.o
 lib-y += ctors.o
 lib-y += ctype.o
 lib-y += list-sort.o
+lib-y += memset.o
 lib-y += muldiv64.o
 lib-y += parse-size.o
 lib-y += rbtree.o
--- /dev/null
+++ b/xen/lib/memset.c
@@ -0,0 +1,33 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * memset - Fill a region of memory with the given value
+ * @s: Pointer to the start of the area.
+ * @c: The byte to fill the area with
+ * @count: The size of the area.
+ *
+ * Do not use memset() to access IO space, use memset_io() instead.
+ */
+void *(memset)(void *s, int c, size_t count)
+{
+	char *xs = (char *) s;
+
+	while (count--)
+		*xs++ = c;
+
+	return s;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 05/23] lib: move memcpy()

[Jan Beulich]

By moving the function into an archive, x86 doesn't need to announce
anymore that is has its own implementation - symbol resolution by the
linker will now guarantee that the generic function remains unused, and
the forwarding to the compiler built-in gets done by the common header
anyway.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -311,27 +311,6 @@ char *(strstr)(const char *s1, const cha
 }
 #endif
 
-#ifndef __HAVE_ARCH_MEMCPY
-/**
- * memcpy - Copy one area of memory to another
- * @dest: Where to copy to
- * @src: Where to copy from
- * @count: The size of the area.
- *
- * 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)
-{
-	char *tmp = (char *) dest, *s = (char *) src;
-
-	while (count--)
-		*tmp++ = *s++;
-
-	return dest;
-}
-#endif
-
 #ifndef __HAVE_ARCH_MEMMOVE
 /**
  * memmove - Copy one area of memory to another
--- a/xen/include/asm-x86/string.h
+++ b/xen/include/asm-x86/string.h
@@ -1,9 +1,6 @@
 #ifndef __X86_STRING_H__
 #define __X86_STRING_H__
 
-#define __HAVE_ARCH_MEMCPY
-#define memcpy(d, s, n) __builtin_memcpy(d, s, n)
-
 #define __HAVE_ARCH_MEMMOVE
 #define memmove(d, s, n) __builtin_memmove(d, s, n)
 
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -4,6 +4,7 @@ lib-y += bsearch.o
 lib-y += ctors.o
 lib-y += ctype.o
 lib-y += list-sort.o
+lib-y += memcpy.o
 lib-y += memset.o
 lib-y += muldiv64.o
 lib-y += parse-size.o
--- /dev/null
+++ b/xen/lib/memcpy.c
@@ -0,0 +1,34 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * memcpy - Copy one area of memory to another
+ * @dest: Where to copy to
+ * @src: Where to copy from
+ * @count: The size of the area.
+ *
+ * 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)
+{
+	char *tmp = (char *) dest, *s = (char *) src;
+
+	while (count--)
+		*tmp++ = *s++;
+
+	return dest;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 06/23] lib: move memmove()

[Jan Beulich]

By moving the function into an archive, x86 doesn't need to announce
anymore that is has its own implementation - symbol resolution by the
linker will now guarantee that the generic function remains unused, and
the forwarding to the compiler built-in gets done by the common header
anyway.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -311,36 +311,6 @@ char *(strstr)(const char *s1, const cha
 }
 #endif
 
-#ifndef __HAVE_ARCH_MEMMOVE
-/**
- * memmove - Copy one area of memory to another
- * @dest: Where to copy to
- * @src: Where to copy from
- * @count: The size of the area.
- *
- * Unlike memcpy(), memmove() copes with overlapping areas.
- */
-void *(memmove)(void *dest, const void *src, size_t count)
-{
-	char *tmp, *s;
-
-	if (dest <= src) {
-		tmp = (char *) dest;
-		s = (char *) src;
-		while (count--)
-			*tmp++ = *s++;
-		}
-	else {
-		tmp = (char *) dest + count;
-		s = (char *) src + count;
-		while (count--)
-			*--tmp = *--s;
-		}
-
-	return dest;
-}
-#endif
-
 #ifndef __HAVE_ARCH_MEMCMP
 /**
  * memcmp - Compare two areas of memory
--- a/xen/include/asm-x86/string.h
+++ b/xen/include/asm-x86/string.h
@@ -1,9 +1,6 @@
 #ifndef __X86_STRING_H__
 #define __X86_STRING_H__
 
-#define __HAVE_ARCH_MEMMOVE
-#define memmove(d, s, n) __builtin_memmove(d, s, n)
-
 #endif /* __X86_STRING_H__ */
 /*
  * Local variables:
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -5,6 +5,7 @@ lib-y += ctors.o
 lib-y += ctype.o
 lib-y += list-sort.o
 lib-y += memcpy.o
+lib-y += memmove.o
 lib-y += memset.o
 lib-y += muldiv64.o
 lib-y += parse-size.o
--- /dev/null
+++ b/xen/lib/memmove.c
@@ -0,0 +1,42 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * memmove - Copy one area of memory to another
+ * @dest: Where to copy to
+ * @src: Where to copy from
+ * @count: The size of the area.
+ *
+ * Unlike memcpy(), memmove() copes with overlapping areas.
+ */
+void *(memmove)(void *dest, const void *src, size_t count)
+{
+	char *tmp, *s;
+
+	if (dest <= src) {
+		tmp = (char *) dest;
+		s = (char *) src;
+		while (count--)
+			*tmp++ = *s++;
+	} else {
+		tmp = (char *) dest + count;
+		s = (char *) src + count;
+		while (count--)
+			*--tmp = *--s;
+	}
+
+	return dest;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 07/23] lib: move memcmp()

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -311,25 +311,6 @@ char *(strstr)(const char *s1, const cha
 }
 #endif
 
-#ifndef __HAVE_ARCH_MEMCMP
-/**
- * memcmp - Compare two areas of memory
- * @cs: One area of memory
- * @ct: Another area of memory
- * @count: The size of the area.
- */
-int (memcmp)(const void *cs, const void *ct, size_t count)
-{
-	const unsigned char *su1, *su2;
-	int res = 0;
-
-	for( su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--)
-		if ((res = *su1 - *su2) != 0)
-			break;
-	return res;
-}
-#endif
-
 #ifndef __HAVE_ARCH_MEMCHR
 /**
  * memchr - Find a character in an area of memory.
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -4,6 +4,7 @@ lib-y += bsearch.o
 lib-y += ctors.o
 lib-y += ctype.o
 lib-y += list-sort.o
+lib-y += memcmp.o
 lib-y += memcpy.o
 lib-y += memmove.o
 lib-y += memset.o
--- /dev/null
+++ b/xen/lib/memcmp.c
@@ -0,0 +1,32 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * memcmp - Compare two areas of memory
+ * @cs: One area of memory
+ * @ct: Another area of memory
+ * @count: The size of the area.
+ */
+int (memcmp)(const void *cs, const void *ct, size_t count)
+{
+	const unsigned char *su1, *su2;
+	int res = 0;
+
+	for( su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--)
+		if ((res = *su1 - *su2) != 0)
+			break;
+	return res;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 08/23] lib: move memchr()

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -311,28 +311,6 @@ char *(strstr)(const char *s1, const cha
 }
 #endif
 
-#ifndef __HAVE_ARCH_MEMCHR
-/**
- * memchr - Find a character in an area of memory.
- * @s: The memory area
- * @c: The byte to search for
- * @n: The size of the area.
- *
- * 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)
-{
-	const unsigned char *p = s;
-
-	while (n--)
-		if ((unsigned char)c == *p++)
-			return (void *)(p - 1);
-
-	return NULL;
-}
-#endif
-
 /**
  * memchr_inv - Find an unmatching character in an area of memory.
  * @s: The memory area
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -4,6 +4,7 @@ lib-y += bsearch.o
 lib-y += ctors.o
 lib-y += ctype.o
 lib-y += list-sort.o
+lib-y += memchr.o
 lib-y += memcmp.o
 lib-y += memcpy.o
 lib-y += memmove.o
--- /dev/null
+++ b/xen/lib/memchr.c
@@ -0,0 +1,35 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * memchr - Find a character in an area of memory.
+ * @s: The memory area
+ * @c: The byte to search for
+ * @n: The size of the area.
+ *
+ * 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)
+{
+	const unsigned char *p = s;
+
+	while (n--)
+		if ((unsigned char)c == *p++)
+			return (void *)(p - 1);
+
+	return NULL;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 09/23] lib: move memchr_inv()

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -311,26 +311,6 @@ char *(strstr)(const char *s1, const cha
 }
 #endif
 
-/**
- * memchr_inv - Find an unmatching character in an area of memory.
- * @s: The memory area
- * @c: The byte that is expected
- * @n: The size of the area.
- *
- * returns the address of the first occurrence of a character other than @c,
- * or %NULL if the whole buffer contains just @c.
- */
-void *memchr_inv(const void *s, int c, size_t n)
-{
-	const unsigned char *p = s;
-
-	while (n--)
-		if ((unsigned char)c != *p++)
-			return (void *)(p - 1);
-
-	return NULL;
-}
-
 /*
  * Local variables:
  * mode: C
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -5,6 +5,7 @@ lib-y += ctors.o
 lib-y += ctype.o
 lib-y += list-sort.o
 lib-y += memchr.o
+lib-y += memchr_inv.o
 lib-y += memcmp.o
 lib-y += memcpy.o
 lib-y += memmove.o
--- /dev/null
+++ b/xen/lib/memchr_inv.c
@@ -0,0 +1,35 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * memchr_inv - Find an unmatching character in an area of memory.
+ * @s: The memory area
+ * @c: The byte that is expected
+ * @n: The size of the area.
+ *
+ * returns the address of the first occurrence of a character other than @c,
+ * or %NULL if the whole buffer contains just @c.
+ */
+void *memchr_inv(const void *s, int c, size_t n)
+{
+	const unsigned char *p = s;
+
+	while (n--)
+		if ((unsigned char)c != *p++)
+			return (void *)(p - 1);
+
+	return NULL;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 10/23] lib: move strlen()

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -184,21 +184,6 @@ char *(strrchr)(const char *s, int c)
 }
 #endif
 
-#ifndef __HAVE_ARCH_STRLEN
-/**
- * strlen - Find the length of a string
- * @s: The string to be sized
- */
-size_t (strlen)(const char * s)
-{
-	const char *sc;
-
-	for (sc = s; *sc != '\0'; ++sc)
-		/* nothing */;
-	return sc - s;
-}
-#endif
-
 #ifndef __HAVE_ARCH_STRNLEN
 /**
  * strnlen - Find the length of a length-limited string
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -14,6 +14,7 @@ lib-y += muldiv64.o
 lib-y += parse-size.o
 lib-y += rbtree.o
 lib-y += sort.o
+lib-y += strlen.o
 lib-$(CONFIG_X86) += xxhash32.o
 lib-$(CONFIG_X86) += xxhash64.o
 
--- /dev/null
+++ b/xen/lib/strlen.c
@@ -0,0 +1,28 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * strlen - Find the length of a string
+ * @s: The string to be sized
+ */
+size_t (strlen)(const char * s)
+{
+	const char *sc;
+
+	for (sc = s; *sc != '\0'; ++sc)
+		/* nothing */;
+	return sc - s;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 11/23] lib: move strnlen()

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -184,22 +184,6 @@ char *(strrchr)(const char *s, int c)
 }
 #endif
 
-#ifndef __HAVE_ARCH_STRNLEN
-/**
- * strnlen - Find the length of a length-limited string
- * @s: The string to be sized
- * @count: The maximum number of bytes to search
- */
-size_t strnlen(const char * s, size_t count)
-{
-	const char *sc;
-
-	for (sc = s; count-- && *sc != '\0'; ++sc)
-		/* nothing */;
-	return sc - s;
-}
-#endif
-
 #ifndef __HAVE_ARCH_STRSPN
 /**
  * strspn - Calculate the length of the initial substring of @s which only
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -15,6 +15,7 @@ lib-y += parse-size.o
 lib-y += rbtree.o
 lib-y += sort.o
 lib-y += strlen.o
+lib-y += strnlen.o
 lib-$(CONFIG_X86) += xxhash32.o
 lib-$(CONFIG_X86) += xxhash64.o
 
--- /dev/null
+++ b/xen/lib/strnlen.c
@@ -0,0 +1,29 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * strnlen - Find the length of a length-limited string
+ * @s: The string to be sized
+ * @count: The maximum number of bytes to search
+ */
+size_t strnlen(const char * s, size_t count)
+{
+	const char *sc;
+
+	for (sc = s; count-- && *sc != '\0'; ++sc)
+		/* nothing */;
+	return sc - s;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 12/23] lib: move strcmp()

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -111,25 +111,6 @@ size_t strlcat(char *dest, const char *s
 EXPORT_SYMBOL(strlcat);
 #endif
 
-#ifndef __HAVE_ARCH_STRCMP
-/**
- * strcmp - Compare two strings
- * @cs: One string
- * @ct: Another string
- */
-int (strcmp)(const char *cs, const char *ct)
-{
-	register signed char __res;
-
-	while (1) {
-		if ((__res = *cs - *ct++) != 0 || !*cs++)
-			break;
-	}
-
-	return __res;
-}
-#endif
-
 #ifndef __HAVE_ARCH_STRNCMP
 /**
  * strncmp - Compare two length-limited strings
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -14,6 +14,7 @@ lib-y += muldiv64.o
 lib-y += parse-size.o
 lib-y += rbtree.o
 lib-y += sort.o
+lib-y += strcmp.o
 lib-y += strlen.o
 lib-y += strnlen.o
 lib-$(CONFIG_X86) += xxhash32.o
--- /dev/null
+++ b/xen/lib/strcmp.c
@@ -0,0 +1,32 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * strcmp - Compare two strings
+ * @cs: One string
+ * @ct: Another string
+ */
+int (strcmp)(const char *cs, const char *ct)
+{
+	register signed char __res;
+
+	while (1) {
+		if ((__res = *cs - *ct++) != 0 || !*cs++)
+			break;
+	}
+
+	return __res;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 13/23] lib: move strncmp()

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -111,27 +111,6 @@ size_t strlcat(char *dest, const char *s
 EXPORT_SYMBOL(strlcat);
 #endif
 
-#ifndef __HAVE_ARCH_STRNCMP
-/**
- * strncmp - Compare two length-limited strings
- * @cs: One string
- * @ct: Another string
- * @count: The maximum number of bytes to compare
- */
-int (strncmp)(const char *cs, const char *ct, size_t count)
-{
-	register signed char __res = 0;
-
-	while (count) {
-		if ((__res = *cs - *ct++) != 0 || !*cs++)
-			break;
-		count--;
-	}
-
-	return __res;
-}
-#endif
-
 #ifndef __HAVE_ARCH_STRCHR
 /**
  * strchr - Find the first occurrence of a character in a string
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -16,6 +16,7 @@ lib-y += rbtree.o
 lib-y += sort.o
 lib-y += strcmp.o
 lib-y += strlen.o
+lib-y += strncmp.o
 lib-y += strnlen.o
 lib-$(CONFIG_X86) += xxhash32.o
 lib-$(CONFIG_X86) += xxhash64.o
--- /dev/null
+++ b/xen/lib/strncmp.c
@@ -0,0 +1,34 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * strncmp - Compare two length-limited strings
+ * @cs: One string
+ * @ct: Another string
+ * @count: The maximum number of bytes to compare
+ */
+int (strncmp)(const char *cs, const char *ct, size_t count)
+{
+	register signed char __res = 0;
+
+	while (count) {
+		if ((__res = *cs - *ct++) != 0 || !*cs++)
+			break;
+		count--;
+	}
+
+	return __res;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 14/23] lib: move strlcpy()

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -56,32 +56,6 @@ int (strcasecmp)(const char *s1, const c
 }
 #endif
 
-#ifndef __HAVE_ARCH_STRLCPY
-/**
- * strlcpy - Copy a %NUL terminated string into a sized buffer
- * @dest: Where to copy the string to
- * @src: Where to copy the string from
- * @size: size of destination buffer
- *
- * Compatible with *BSD: the result is always a valid
- * NUL-terminated string that fits in the buffer (unless,
- * of course, the buffer size is zero). It does not pad
- * out the result like strncpy() does.
- */
-size_t strlcpy(char *dest, const char *src, size_t size)
-{
-	size_t ret = strlen(src);
-
-	if (size) {
-		size_t len = (ret >= size) ? size-1 : ret;
-		memcpy(dest, src, len);
-		dest[len] = '\0';
-	}
-	return ret;
-}
-EXPORT_SYMBOL(strlcpy);
-#endif
-
 #ifndef __HAVE_ARCH_STRLCAT
 /**
  * strlcat - Append a %NUL terminated string into a sized buffer
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -15,6 +15,7 @@ lib-y += parse-size.o
 lib-y += rbtree.o
 lib-y += sort.o
 lib-y += strcmp.o
+lib-y += strlcpy.o
 lib-y += strlen.o
 lib-y += strncmp.o
 lib-y += strnlen.o
--- /dev/null
+++ b/xen/lib/strlcpy.c
@@ -0,0 +1,38 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * strlcpy - Copy a %NUL terminated string into a sized buffer
+ * @dest: Where to copy the string to
+ * @src: Where to copy the string from
+ * @size: size of destination buffer
+ *
+ * Compatible with *BSD: the result is always a valid
+ * NUL-terminated string that fits in the buffer (unless,
+ * of course, the buffer size is zero). It does not pad
+ * out the result like strncpy() does.
+ */
+size_t strlcpy(char *dest, const char *src, size_t size)
+{
+	size_t ret = strlen(src);
+
+	if (size) {
+		size_t len = (ret >= size) ? size-1 : ret;
+		memcpy(dest, src, len);
+		dest[len] = '\0';
+	}
+	return ret;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 15/23] lib: move strlcat()

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -56,35 +56,6 @@ int (strcasecmp)(const char *s1, const c
 }
 #endif
 
-#ifndef __HAVE_ARCH_STRLCAT
-/**
- * strlcat - Append a %NUL terminated string into a sized buffer
- * @dest: Where to copy the string to
- * @src: Where to copy the string from
- * @size: size of destination buffer
- *
- * Compatible with *BSD: the result is always a valid
- * NUL-terminated string that fits in the buffer (unless,
- * of course, the buffer size is zero).
- */
-size_t strlcat(char *dest, const char *src, size_t size)
-{
-	size_t slen = strlen(src);
-	size_t dlen = strnlen(dest, size);
-	char *p = dest + dlen;
-
-	while ((p - dest) < size)
-		if ((*p++ = *src++) == '\0')
-			break;
-
-	if (dlen < size)
-		*(p-1) = '\0';
-
-	return slen + dlen;
-}
-EXPORT_SYMBOL(strlcat);
-#endif
-
 #ifndef __HAVE_ARCH_STRCHR
 /**
  * strchr - Find the first occurrence of a character in a string
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -15,6 +15,7 @@ lib-y += parse-size.o
 lib-y += rbtree.o
 lib-y += sort.o
 lib-y += strcmp.o
+lib-y += strlcat.o
 lib-y += strlcpy.o
 lib-y += strlen.o
 lib-y += strncmp.o
--- /dev/null
+++ b/xen/lib/strlcat.c
@@ -0,0 +1,41 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * strlcat - Append a %NUL terminated string into a sized buffer
+ * @dest: Where to copy the string to
+ * @src: Where to copy the string from
+ * @size: size of destination buffer
+ *
+ * Compatible with *BSD: the result is always a valid
+ * NUL-terminated string that fits in the buffer (unless,
+ * of course, the buffer size is zero).
+ */
+size_t strlcat(char *dest, const char *src, size_t size)
+{
+	size_t slen = strlen(src);
+	size_t dlen = strnlen(dest, size);
+	char *p = dest + dlen;
+
+	while ((p - dest) < size)
+		if ((*p++ = *src++) == '\0')
+			break;
+
+	if (dlen < size)
+		*(p-1) = '\0';
+
+	return slen + dlen;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 16/23] lib: move strchr()

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -56,21 +56,6 @@ int (strcasecmp)(const char *s1, const c
 }
 #endif
 
-#ifndef __HAVE_ARCH_STRCHR
-/**
- * strchr - Find the first occurrence of a character in a string
- * @s: The string to be searched
- * @c: The character to search for
- */
-char *(strchr)(const char *s, int c)
-{
-	for(; *s != (char) c; ++s)
-		if (*s == '\0')
-			return NULL;
-	return (char *) s;
-}
-#endif
-
 #ifndef __HAVE_ARCH_STRRCHR
 /**
  * strrchr - Find the last occurrence of a character in a string
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -14,6 +14,7 @@ lib-y += muldiv64.o
 lib-y += parse-size.o
 lib-y += rbtree.o
 lib-y += sort.o
+lib-y += strchr.o
 lib-y += strcmp.o
 lib-y += strlcat.o
 lib-y += strlcpy.o
--- /dev/null
+++ b/xen/lib/strchr.c
@@ -0,0 +1,28 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * strchr - Find the first occurrence of a character in a string
+ * @s: The string to be searched
+ * @c: The character to search for
+ */
+char *(strchr)(const char *s, int c)
+{
+	for(; *s != (char) c; ++s)
+		if (*s == '\0')
+			return NULL;
+	return (char *) s;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 17/23] lib: move strrchr()

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -56,24 +56,6 @@ int (strcasecmp)(const char *s1, const c
 }
 #endif
 
-#ifndef __HAVE_ARCH_STRRCHR
-/**
- * strrchr - Find the last occurrence of a character in a string
- * @s: The string to be searched
- * @c: The character to search for
- */
-char *(strrchr)(const char *s, int c)
-{
-	const char *p = s + strlen(s);
-
-	for (; *p != (char)c; --p)
-		if (p == s)
-			return NULL;
-
-	return (char *)p;
-}
-#endif
-
 #ifndef __HAVE_ARCH_STRSPN
 /**
  * strspn - Calculate the length of the initial substring of @s which only
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -21,6 +21,7 @@ lib-y += strlcpy.o
 lib-y += strlen.o
 lib-y += strncmp.o
 lib-y += strnlen.o
+lib-y += strrchr.o
 lib-$(CONFIG_X86) += xxhash32.o
 lib-$(CONFIG_X86) += xxhash64.o
 
--- /dev/null
+++ b/xen/lib/strrchr.c
@@ -0,0 +1,31 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * strrchr - Find the last occurrence of a character in a string
+ * @s: The string to be searched
+ * @c: The character to search for
+ */
+char *(strrchr)(const char *s, int c)
+{
+	const char *p = s + strlen(s);
+
+	for (; *p != (char)c; --p)
+		if (p == s)
+			return NULL;
+
+	return (char *)p;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 18/23] lib: move strstr()

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -131,27 +131,6 @@ char * strsep(char **s, const char *ct)
 }
 #endif
 
-#ifndef __HAVE_ARCH_STRSTR
-/**
- * strstr - Find the first substring in a %NUL terminated string
- * @s1: The string to be searched
- * @s2: The string to search for
- */
-char *(strstr)(const char *s1, const char *s2)
-{
-	size_t l1, l2 = strlen(s2);
-
-	if (!l2)
-		return (char *)s1;
-
-	for (l1 = strlen(s1); l1 >= l2; --l1, ++s1)
-		if (!memcmp(s1, s2, l2))
-			return (char *)s1;
-
-	return NULL;
-}
-#endif
-
 /*
  * Local variables:
  * mode: C
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -22,6 +22,7 @@ lib-y += strlen.o
 lib-y += strncmp.o
 lib-y += strnlen.o
 lib-y += strrchr.o
+lib-y += strstr.o
 lib-$(CONFIG_X86) += xxhash32.o
 lib-$(CONFIG_X86) += xxhash64.o
 
--- /dev/null
+++ b/xen/lib/strstr.c
@@ -0,0 +1,34 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * strstr - Find the first substring in a %NUL terminated string
+ * @s1: The string to be searched
+ * @s2: The string to search for
+ */
+char *(strstr)(const char *s1, const char *s2)
+{
+	size_t l1, l2 = strlen(s2);
+
+	if (!l2)
+		return (char *)s1;
+
+	for (l1 = strlen(s1); l1 >= l2; --l1, ++s1)
+		if (!memcmp(s1, s2, l2))
+			return (char *)s1;
+
+	return NULL;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 19/23] lib: move strcasecmp()

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -41,21 +41,6 @@ int strnicmp(const char *s1, const char
 }
 #endif
 
-#ifndef __HAVE_ARCH_STRCASECMP
-int (strcasecmp)(const char *s1, const char *s2)
-{
-    int c1, c2;
-
-    do
-    {
-        c1 = tolower(*s1++);
-        c2 = tolower(*s2++);
-    } while ( c1 == c2 && c1 != 0 );
-
-    return c1 - c2;
-}
-#endif
-
 #ifndef __HAVE_ARCH_STRSPN
 /**
  * strspn - Calculate the length of the initial substring of @s which only
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -14,6 +14,7 @@ lib-y += muldiv64.o
 lib-y += parse-size.o
 lib-y += rbtree.o
 lib-y += sort.o
+lib-y += strcasecmp.o
 lib-y += strchr.o
 lib-y += strcmp.o
 lib-y += strlcat.o
--- /dev/null
+++ b/xen/lib/strcasecmp.c
@@ -0,0 +1,29 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+#include <xen/ctype.h>
+
+int (strcasecmp)(const char *s1, const char *s2)
+{
+    int c1, c2;
+
+    do
+    {
+        c1 = tolower(*s1++);
+        c2 = tolower(*s2++);
+    } while ( c1 == c2 && c1 != 0 );
+
+    return c1 - c2;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 20/23] lib: move/rename strnicmp() to strncasecmp()

[Jan Beulich]

While moving the implementation, also rename it to match strcasecmp(),
allowing the similar use of a compiler builting in this case as well.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1223,8 +1223,8 @@ static int __init map_range_to_domain(co
      * They are not MMIO and therefore a domain should not be able to
      * manage them via the IOMEM interface.
      */
-    if ( strnicmp(dt_node_full_name(dev), "/reserved-memory/",
-         strlen("/reserved-memory/")) != 0 )
+    if ( strncasecmp(dt_node_full_name(dev), "/reserved-memory/",
+                     strlen("/reserved-memory/")) != 0 )
     {
         res = iomem_permit_access(d, paddr_to_pfn(addr),
                 paddr_to_pfn(PAGE_ALIGN(addr + len - 1)));
--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -8,39 +8,6 @@
 #include <xen/string.h>
 #include <xen/ctype.h>
 
-#ifndef __HAVE_ARCH_STRNICMP
-/**
- * strnicmp - Case insensitive, length-limited string comparison
- * @s1: One string
- * @s2: The other string
- * @len: the maximum number of characters to compare
- */
-int strnicmp(const char *s1, const char *s2, size_t len)
-{
-	/* Yes, Virginia, it had better be unsigned */
-	unsigned char c1, c2;
-
-	c1 = 0;	c2 = 0;
-	if (len) {
-		do {
-			c1 = *s1; c2 = *s2;
-			s1++; s2++;
-			if (!c1)
-				break;
-			if (!c2)
-				break;
-			if (c1 == c2)
-				continue;
-			c1 = tolower(c1);
-			c2 = tolower(c2);
-			if (c1 != c2)
-				break;
-		} while (--len);
-	}
-	return (int)c1 - (int)c2;
-}
-#endif
-
 #ifndef __HAVE_ARCH_STRSPN
 /**
  * strspn - Calculate the length of the initial substring of @s which only
--- a/xen/drivers/acpi/pmstat.c
+++ b/xen/drivers/acpi/pmstat.c
@@ -275,14 +275,14 @@ static int get_cpufreq_para(struct xen_s
         strlcpy(op->u.get_para.scaling_governor, "Unknown", CPUFREQ_NAME_LEN);
 
     /* governor specific para */
-    if ( !strnicmp(op->u.get_para.scaling_governor, 
-                   "userspace", CPUFREQ_NAME_LEN) )
+    if ( !strncasecmp(op->u.get_para.scaling_governor,
+                      "userspace", CPUFREQ_NAME_LEN) )
     {
         op->u.get_para.u.userspace.scaling_setspeed = policy->cur;
     }
 
-    if ( !strnicmp(op->u.get_para.scaling_governor, 
-                   "ondemand", CPUFREQ_NAME_LEN) )
+    if ( !strncasecmp(op->u.get_para.scaling_governor,
+                      "ondemand", CPUFREQ_NAME_LEN) )
     {
         ret = get_cpufreq_ondemand_para(
             &op->u.get_para.u.ondemand.sampling_rate_max,
@@ -350,8 +350,8 @@ static int set_cpufreq_para(struct xen_s
     {
         unsigned int freq =op->u.set_para.ctrl_value;
 
-        if ( !strnicmp(policy->governor->name,
-                       "userspace", CPUFREQ_NAME_LEN) )
+        if ( !strncasecmp(policy->governor->name,
+                          "userspace", CPUFREQ_NAME_LEN) )
             ret = write_userspace_scaling_setspeed(op->cpuid, freq);
         else
             ret = -EINVAL;
@@ -363,8 +363,8 @@ static int set_cpufreq_para(struct xen_s
     {
         unsigned int sampling_rate = op->u.set_para.ctrl_value;
 
-        if ( !strnicmp(policy->governor->name,
-                       "ondemand", CPUFREQ_NAME_LEN) )
+        if ( !strncasecmp(policy->governor->name,
+                          "ondemand", CPUFREQ_NAME_LEN) )
             ret = write_ondemand_sampling_rate(sampling_rate);
         else
             ret = -EINVAL;
@@ -376,8 +376,8 @@ static int set_cpufreq_para(struct xen_s
     {
         unsigned int up_threshold = op->u.set_para.ctrl_value;
 
-        if ( !strnicmp(policy->governor->name,
-                       "ondemand", CPUFREQ_NAME_LEN) )
+        if ( !strncasecmp(policy->governor->name,
+                          "ondemand", CPUFREQ_NAME_LEN) )
             ret = write_ondemand_up_threshold(up_threshold);
         else
             ret = -EINVAL;
--- a/xen/drivers/cpufreq/cpufreq.c
+++ b/xen/drivers/cpufreq/cpufreq.c
@@ -111,7 +111,7 @@ struct cpufreq_governor *__find_governor
         return NULL;
 
     list_for_each_entry(t, &cpufreq_governor_list, governor_list)
-        if (!strnicmp(governor, t->name, CPUFREQ_NAME_LEN))
+        if (!strncasecmp(governor, t->name, CPUFREQ_NAME_LEN))
             return t;
 
     return NULL;
--- a/xen/include/xen/string.h
+++ b/xen/include/xen/string.h
@@ -16,8 +16,8 @@ size_t strlcpy(char *, const char *, siz
 size_t strlcat(char *, const char *, size_t);
 int strcmp(const char *, const char *);
 int strncmp(const char *, const char *, size_t);
-int strnicmp(const char *, const char *, size_t);
 int strcasecmp(const char *, const char *);
+int strncasecmp(const char *, const char *, size_t);
 char *strchr(const char *, int);
 char *strrchr(const char *, int);
 char *strstr(const char *, const char *);
@@ -48,6 +48,10 @@ void *memchr_inv(const void *, int, size
 #define strcasecmp(s1, s2) __builtin_strcasecmp(s1, s2)
 #endif
 
+#ifndef __HAVE_ARCH_STRCASECMP
+#define strncasecmp(s1, s2, n) __builtin_strncasecmp(s1, s2, n)
+#endif
+
 #ifndef __HAVE_ARCH_STRCHR
 #define strchr(s1, c) __builtin_strchr(s1, c)
 #endif
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -20,6 +20,7 @@ lib-y += strcmp.o
 lib-y += strlcat.o
 lib-y += strlcpy.o
 lib-y += strlen.o
+lib-y += strncasecmp.o
 lib-y += strncmp.o
 lib-y += strnlen.o
 lib-y += strrchr.o
--- /dev/null
+++ b/xen/lib/strncasecmp.c
@@ -0,0 +1,47 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+#include <xen/ctype.h>
+
+/**
+ * strncasecmp - Case insensitive, length-limited string comparison
+ * @s1: One string
+ * @s2: The other string
+ * @len: the maximum number of characters to compare
+ */
+int (strncasecmp)(const char *s1, const char *s2, size_t len)
+{
+	/* Yes, Virginia, it had better be unsigned */
+	unsigned char c1, c2;
+
+	c1 = 0;	c2 = 0;
+	if (len) {
+		do {
+			c1 = *s1; c2 = *s2;
+			s1++; s2++;
+			if (!c1)
+				break;
+			if (!c2)
+				break;
+			if (c1 == c2)
+				continue;
+			c1 = tolower(c1);
+			c2 = tolower(c2);
+			if (c1 != c2)
+				break;
+		} while (--len);
+	}
+	return (int)c1 - (int)c2;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 21/23] lib: move strspn()

[Jan Beulich]

In fact the function is unused at present, and hence will now get
omitted from the final binaries.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -8,33 +8,6 @@
 #include <xen/string.h>
 #include <xen/ctype.h>
 
-#ifndef __HAVE_ARCH_STRSPN
-/**
- * strspn - Calculate the length of the initial substring of @s which only
- * 	contain letters in @accept
- * @s: The string to be searched
- * @accept: The string to search for
- */
-size_t strspn(const char *s, const char *accept)
-{
-	const char *p;
-	const char *a;
-	size_t count = 0;
-
-	for (p = s; *p != '\0'; ++p) {
-		for (a = accept; *a != '\0'; ++a) {
-			if (*p == *a)
-				break;
-		}
-		if (*a == '\0')
-			return count;
-		++count;
-	}
-
-	return count;
-}
-#endif
-
 #ifndef __HAVE_ARCH_STRPBRK
 /**
  * strpbrk - Find the first occurrence of a set of characters
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -24,6 +24,7 @@ lib-y += strncasecmp.o
 lib-y += strncmp.o
 lib-y += strnlen.o
 lib-y += strrchr.o
+lib-y += strspn.o
 lib-y += strstr.o
 lib-$(CONFIG_X86) += xxhash32.o
 lib-$(CONFIG_X86) += xxhash64.o
--- /dev/null
+++ b/xen/lib/strspn.c
@@ -0,0 +1,40 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * strspn - Calculate the length of the initial substring of @s which only
+ * 	contain letters in @accept
+ * @s: The string to be searched
+ * @accept: The string to search for
+ */
+size_t strspn(const char *s, const char *accept)
+{
+	const char *p;
+	const char *a;
+	size_t count = 0;
+
+	for (p = s; *p != '\0'; ++p) {
+		for (a = accept; *a != '\0'; ++a) {
+			if (*p == *a)
+				break;
+		}
+		if (*a == '\0')
+			return count;
+		++count;
+	}
+
+	return count;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 22/23] lib: move strpbrk()

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -8,26 +8,6 @@
 #include <xen/string.h>
 #include <xen/ctype.h>
 
-#ifndef __HAVE_ARCH_STRPBRK
-/**
- * strpbrk - Find the first occurrence of a set of characters
- * @cs: The string to be searched
- * @ct: The characters to search for
- */
-char * strpbrk(const char * cs,const char * ct)
-{
-	const char *sc1,*sc2;
-
-	for( sc1 = cs; *sc1 != '\0'; ++sc1) {
-		for( sc2 = ct; *sc2 != '\0'; ++sc2) {
-			if (*sc1 == *sc2)
-				return (char *) sc1;
-		}
-	}
-	return NULL;
-}
-#endif
-
 #ifndef __HAVE_ARCH_STRSEP
 /**
  * strsep - Split a string into tokens
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -23,6 +23,7 @@ lib-y += strlen.o
 lib-y += strncasecmp.o
 lib-y += strncmp.o
 lib-y += strnlen.o
+lib-y += strpbrk.o
 lib-y += strrchr.o
 lib-y += strspn.o
 lib-y += strstr.o
--- /dev/null
+++ b/xen/lib/strpbrk.c
@@ -0,0 +1,33 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * strpbrk - Find the first occurrence of a set of characters
+ * @cs: The string to be searched
+ * @ct: The characters to search for
+ */
+char *strpbrk(const char * cs,const char * ct)
+{
+	const char *sc1,*sc2;
+
+	for( sc1 = cs; *sc1 != '\0'; ++sc1) {
+		for( sc2 = ct; *sc2 != '\0'; ++sc2) {
+			if (*sc1 == *sc2)
+				return (char *) sc1;
+		}
+	}
+	return NULL;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

[PATCH 23/23] lib: move strsep()

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -40,7 +40,6 @@ obj-y += softirq.o
 obj-y += smp.o
 obj-y += spinlock.o
 obj-y += stop_machine.o
-obj-y += string.o
 obj-y += symbols.o
 obj-y += tasklet.o
 obj-y += time.o
--- a/xen/common/string.c
+++ /dev/null
@@ -1,47 +0,0 @@
-/*
- *  linux/lib/string.c
- *
- *  Copyright (C) 1991, 1992  Linus Torvalds
- */
-
-#include <xen/types.h>
-#include <xen/string.h>
-#include <xen/ctype.h>
-
-#ifndef __HAVE_ARCH_STRSEP
-/**
- * strsep - Split a string into tokens
- * @s: The string to be searched
- * @ct: The characters to search for
- *
- * strsep() updates @s to point after the token, ready for the next call.
- *
- * It returns empty tokens, too, behaving exactly like the libc function
- * of that name. In fact, it was stolen from glibc2 and de-fancy-fied.
- * Same semantics, slimmer shape. ;)
- */
-char * strsep(char **s, const char *ct)
-{
-	char *sbegin = *s, *end;
-
-	if (sbegin == NULL)
-		return NULL;
-
-	end = strpbrk(sbegin, ct);
-	if (end)
-		*end++ = '\0';
-	*s = end;
-
-	return sbegin;
-}
-#endif
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 8
- * tab-width: 8
- * indent-tabs-mode: t
- * End:
- */
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -25,6 +25,7 @@ lib-y += strncmp.o
 lib-y += strnlen.o
 lib-y += strpbrk.o
 lib-y += strrchr.o
+lib-y += strsep.o
 lib-y += strspn.o
 lib-y += strstr.o
 lib-$(CONFIG_X86) += xxhash32.o
--- /dev/null
+++ b/xen/lib/strsep.c
@@ -0,0 +1,41 @@
+/*
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <xen/string.h>
+
+/**
+ * strsep - Split a string into tokens
+ * @s: The string to be searched
+ * @ct: The characters to search for
+ *
+ * strsep() updates @s to point after the token, ready for the next call.
+ *
+ * It returns empty tokens, too, behaving exactly like the libc function
+ * of that name. In fact, it was stolen from glibc2 and de-fancy-fied.
+ * Same semantics, slimmer shape. ;)
+ */
+char *strsep(char **s, const char *ct)
+{
+	char *sbegin = *s, *end;
+
+	if (sbegin == NULL)
+		return NULL;
+
+	end = strpbrk(sbegin, ct);
+	if (end)
+		*end++ = '\0';
+	*s = end;
+
+	return sbegin;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ */

Re: [PATCH 00/23] further population of xen/lib/

[Julien Grall]

Hi Jan,

On 01/04/2021 11:14, Jan Beulich wrote:
> This is to dissolve / move xen/common/lib.c and xen/common/string.c.
> One benefit of moving these functions into an archive is that we can
> drop some of the related __HAVE_ARCH_* #define-s: By living in an
> archive, the per-arch functions will preempt any loading of the
> respective functions (objects) from the archive. (Down the road we
> may want to move the per-arch functions into archives as well, at
> which point the per-arch archive(s) would need to be specified ahead
> of the common one(s) to the linker.)

While I think it is a good idea to move code in xen/lib, I am not 
convinced that having a single function per file is that beneficial.

Do you have numbers showing how much Xen will shrink after this series?

Cheers,

-- 
Julien Grall

Re: [PATCH 00/23] further population of xen/lib/

[Jan Beulich]

On 01.04.2021 13:54, Julien Grall wrote:
> On 01/04/2021 11:14, Jan Beulich wrote:
>> This is to dissolve / move xen/common/lib.c and xen/common/string.c.
>> One benefit of moving these functions into an archive is that we can
>> drop some of the related __HAVE_ARCH_* #define-s: By living in an
>> archive, the per-arch functions will preempt any loading of the
>> respective functions (objects) from the archive. (Down the road we
>> may want to move the per-arch functions into archives as well, at
>> which point the per-arch archive(s) would need to be specified ahead
>> of the common one(s) to the linker.)
> 
> While I think it is a good idea to move code in xen/lib, I am not 
> convinced that having a single function per file is that beneficial.
> 
> Do you have numbers showing how much Xen will shrink after this series?

In the default build, from all I was able to tell, there's one function
that's unused (strspn(), as mentioned in the respective patch description).
I don't think I've been claiming any space savings here, though, so I
wonder why you make this a criteria at all. The functions being one per
CU is such that they can be individually overridden by an arch, without
pulling in dead code.

Jan

Re: [PATCH 00/23] further population of xen/lib/

[Julien Grall]

Hi Jan,

On 01/04/2021 14:43, Jan Beulich wrote:
> On 01.04.2021 13:54, Julien Grall wrote:
>> On 01/04/2021 11:14, Jan Beulich wrote:
>>> This is to dissolve / move xen/common/lib.c and xen/common/string.c.
>>> One benefit of moving these functions into an archive is that we can
>>> drop some of the related __HAVE_ARCH_* #define-s: By living in an
>>> archive, the per-arch functions will preempt any loading of the
>>> respective functions (objects) from the archive. (Down the road we
>>> may want to move the per-arch functions into archives as well, at
>>> which point the per-arch archive(s) would need to be specified ahead
>>> of the common one(s) to the linker.)
>>
>> While I think it is a good idea to move code in xen/lib, I am not
>> convinced that having a single function per file is that beneficial.
>>
>> Do you have numbers showing how much Xen will shrink after this series?
> 
> In the default build, from all I was able to tell, there's one function
> that's unused (strspn(), as mentioned in the respective patch description).
> I don't think I've been claiming any space savings here, though, so I

You didn't. I was trying to guess why you wrote this series given that 
your cover letter doesn't provide a lot of benefits (other than dropping 
__HAVE_ARCH_*).

> wonder why you make this a criteria at all.

Because this is the main reason I would be willing to ack this series. 
This outweight the increase number of files with just a single function 
implemented.

> The functions being one per
> CU is such that they can be individually overridden by an arch, without
> pulling in dead code.

I would agree with functions like memcpy/memset() because you can gain a 
lot to outweight the implementation in assembly. I am not convinced this 
would be true for functions such as strlen().

So overall, the number of functions requiring overriding will likely be 
pretty limited and #ifdef would be IMHO tolerable.

Although, I would be OK with creating a file per function that are 
already overrided. For all the others, I think this is just pointless.

Cheers,

-- 
Julien Grall

Re: [PATCH 00/23] further population of xen/lib/

[Jan Beulich]

On 01.04.2021 16:04, Julien Grall wrote:
> Hi Jan,
> 
> On 01/04/2021 14:43, Jan Beulich wrote:
>> On 01.04.2021 13:54, Julien Grall wrote:
>>> On 01/04/2021 11:14, Jan Beulich wrote:
>>>> This is to dissolve / move xen/common/lib.c and xen/common/string.c.
>>>> One benefit of moving these functions into an archive is that we can
>>>> drop some of the related __HAVE_ARCH_* #define-s: By living in an
>>>> archive, the per-arch functions will preempt any loading of the
>>>> respective functions (objects) from the archive. (Down the road we
>>>> may want to move the per-arch functions into archives as well, at
>>>> which point the per-arch archive(s) would need to be specified ahead
>>>> of the common one(s) to the linker.)
>>>
>>> While I think it is a good idea to move code in xen/lib, I am not
>>> convinced that having a single function per file is that beneficial.
>>>
>>> Do you have numbers showing how much Xen will shrink after this series?
>>
>> In the default build, from all I was able to tell, there's one function
>> that's unused (strspn(), as mentioned in the respective patch description).
>> I don't think I've been claiming any space savings here, though, so I
> 
> You didn't. I was trying to guess why you wrote this series given that 
> your cover letter doesn't provide a lot of benefits (other than dropping 
> __HAVE_ARCH_*).
> 
>> wonder why you make this a criteria at all.
> 
> Because this is the main reason I would be willing to ack this series. 
> This outweight the increase number of files with just a single function 
> implemented.
> 
>> The functions being one per
>> CU is such that they can be individually overridden by an arch, without
>> pulling in dead code.
> 
> I would agree with functions like memcpy/memset() because you can gain a 
> lot to outweight the implementation in assembly. I am not convinced this 
> would be true for functions such as strlen().

strlen() is actually a pretty good candidate for overriding, and
we may even want to on x86 with newer hardware's "Fast Short REP
CMPSB/SCASB".

> So overall, the number of functions requiring overriding will likely be 
> pretty limited and #ifdef would be IMHO tolerable.
> 
> Although, I would be OK with creating a file per function that are 
> already overrided. For all the others, I think this is just pointless.

Well, I don't see a reason to special case individual functions.
Plus any reasonable static library should imo have one (global)
function per object file in the normal case; there may be very
few exceptions. Drawing an ad hoc boundary at what currently has
an override somewhere doesn't look very attractive to me. Plus
to be honest while I would find it unfair to ask to further
split things if I did just a partial conversion (i.e. invest yet
more time), I find it rather odd to be asked to undo some of the
splitting when I've already taken the extra time to make things
consistent.

Jan

Re: [PATCH 00/23] further population of xen/lib/

[Julien Grall]

On 01/04/2021 15:27, Jan Beulich wrote:
> On 01.04.2021 16:04, Julien Grall wrote: >> So overall, the number of functions requiring overriding will likely be
>> pretty limited and #ifdef would be IMHO tolerable.
>>
>> Although, I would be OK with creating a file per function that are
>> already overrided. For all the others, I think this is just pointless.
> 
> Well, I don't see a reason to special case individual functions.
> Plus any reasonable static library should imo have one (global)
> function per object file in the normal case; there may be very
> few exceptions. Drawing an ad hoc boundary at what currently has
> an override somewhere doesn't look very attractive to me. Plus
> to be honest while I would find it unfair to ask to further
> split things if I did just a partial conversion (i.e. invest yet
> more time), I find it rather odd to be asked to undo some of the
> splitting when I've already taken the extra time to make things
> consistent.

I am sure each of us spent time working on a solution that some 
reviewers were not necessary convinced of the usefulness and they had to 
undo the series...

In this case, you sent a large series with close to 0 commit message + a 
small cover letter. So I think it is fair for a reviewer to be 
unconvinced and ask for more input.

You provided that now, I think we want a short summary (or a link to the 
conversation) in each commit message.

This will be helpful to understand why the move was made without having 
to spend ages finding the original discussion.

Cheers,

-- 
Julien Grall

Re: [PATCH 00/23] further population of xen/lib/

[Jan Beulich]

On 01.04.2021 16:55, Julien Grall wrote:
> 
> 
> On 01/04/2021 15:27, Jan Beulich wrote:
>> On 01.04.2021 16:04, Julien Grall wrote: >> So overall, the number of functions requiring overriding will likely be
>>> pretty limited and #ifdef would be IMHO tolerable.
>>>
>>> Although, I would be OK with creating a file per function that are
>>> already overrided. For all the others, I think this is just pointless.
>>
>> Well, I don't see a reason to special case individual functions.
>> Plus any reasonable static library should imo have one (global)
>> function per object file in the normal case; there may be very
>> few exceptions. Drawing an ad hoc boundary at what currently has
>> an override somewhere doesn't look very attractive to me. Plus
>> to be honest while I would find it unfair to ask to further
>> split things if I did just a partial conversion (i.e. invest yet
>> more time), I find it rather odd to be asked to undo some of the
>> splitting when I've already taken the extra time to make things
>> consistent.
> 
> I am sure each of us spent time working on a solution that some 
> reviewers were not necessary convinced of the usefulness and they had to 
> undo the series...
> 
> In this case, you sent a large series with close to 0 commit message + a 
> small cover letter. So I think it is fair for a reviewer to be 
> unconvinced and ask for more input.
> 
> You provided that now, I think we want a short summary (or a link to the 
> conversation) in each commit message.
> 
> This will be helpful to understand why the move was made without having 
> to spend ages finding the original discussion.

I'll add "Allow the function to be individually linkable, discardable,
and overridable." to all the str*() and mem*() patch descriptions. Is
that going to be good enough?

Jan

Re: [PATCH 00/23] further population of xen/lib/

[Julien Grall]

On 01/04/2021 16:25, Jan Beulich wrote:
> On 01.04.2021 16:55, Julien Grall wrote:
>>
>>
>> On 01/04/2021 15:27, Jan Beulich wrote:
>>> On 01.04.2021 16:04, Julien Grall wrote: >> So overall, the number of functions requiring overriding will likely be
>>>> pretty limited and #ifdef would be IMHO tolerable.
>>>>
>>>> Although, I would be OK with creating a file per function that are
>>>> already overrided. For all the others, I think this is just pointless.
>>>
>>> Well, I don't see a reason to special case individual functions.
>>> Plus any reasonable static library should imo have one (global)
>>> function per object file in the normal case; there may be very
>>> few exceptions. Drawing an ad hoc boundary at what currently has
>>> an override somewhere doesn't look very attractive to me. Plus
>>> to be honest while I would find it unfair to ask to further
>>> split things if I did just a partial conversion (i.e. invest yet
>>> more time), I find it rather odd to be asked to undo some of the
>>> splitting when I've already taken the extra time to make things
>>> consistent.
>>
>> I am sure each of us spent time working on a solution that some
>> reviewers were not necessary convinced of the usefulness and they had to
>> undo the series...
>>
>> In this case, you sent a large series with close to 0 commit message + a
>> small cover letter. So I think it is fair for a reviewer to be
>> unconvinced and ask for more input.
>>
>> You provided that now, I think we want a short summary (or a link to the
>> conversation) in each commit message.
>>
>> This will be helpful to understand why the move was made without having
>> to spend ages finding the original discussion.
> 
> I'll add "Allow the function to be individually linkable, discardable,
> and overridable." to all the str*() and mem*() patch descriptions. Is
> that going to be good enough?
It will be good for me.

Cheers,

> 
> Jan
> 

-- 
Julien Grall