[U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address

Alexander Holler holler at ahsoftware.de
Mon May 23 23:07:23 CEST 2011


Hello,

Am 23.05.2011 11:03, schrieb Matthias Weisser:
> In some cases (e.g. bootm with a elf payload which is already at the right
> position) there is a in place copy of data to the same address. Catching this
> saves some ms while booting.
>
> Signed-off-by: Matthias Weisser<weisserm at arcor.de>
> ---
> Changes since V1:
>    - Made subject more informative
>    - Removed the optimization from bcopy as bcopy is not used anywhere
>
>   lib/string.c |    6 ++++++
>   1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/lib/string.c b/lib/string.c
> index b375b81..2c4f0ec 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -467,6 +467,9 @@ void * memcpy(void *dest, const void *src, size_t count)
>   	unsigned long *dl = (unsigned long *)dest, *sl = (unsigned long *)src;
>   	char *d8, *s8;
>
> +	if (src == dest)
> +		return dest;
> +

here is the same, as in the patch I've commented before. There exist no 
reason to add a check for identity to memcpy() because memcpy doesn't 
support overlapping regions (and identity is just a special case of 
overlapping regions). If something might call memcpy() with overlapping 
or identical regions, it should use memmove().

>   	/* while all data is aligned (common case), copy a word at a time */
>   	if ( (((ulong)dest | (ulong)src)&  (sizeof(*dl) - 1)) == 0) {
>   		while (count>= sizeof(*dl)) {
> @@ -497,6 +500,9 @@ void * memmove(void * dest,const void *src,size_t count)
>   {
>   	char *tmp, *s;
>
> +	if (src == dest)
> +		return dest;
> +
>   	if (dest<= src) {

Here it is ok, but the check <= could be modified to < too.

Just to clarify my reasoning: the only reason why memcpy() exists, is 
because it should have been a faster version of memmove() without the 
necessary checks.
So if a bug proof of version of memcpy() is wanted, there is no need to 
have a different implementation for memcpy() and memcpy() could just be 
an alias for memmove().
But adding a check for identity to memcpy() is unnecessary.

Sorry, but I had to comment this after having read to many comments in a 
bug about something similiar in

https://bugzilla.redhat.com/show_bug.cgi?id=638477

(Be aware, reading that bug might hurt your brain)

Regards,

Alexander


More information about the U-Boot mailing list