[U-Boot] [PATCH V3 1/3] lib_generic memcpy: copy one word at a time if possible

Chris Moore
2009-10-10T06:13:47Z

Alessandro Rubini a ?crit :
> From: Alessandro Rubini <rubini@unipv.it>
>
> If source and destination are aligned, this copies ulong values
> until possible, trailing part is copied by byte. Thanks for the details
> to Wolfgang Denk, Mike Frysinger, Peter Tyser, Chris Moore.
>
> Signed-off-by: Alessandro Rubini <rubini@unipv.it>
> Acked-by: Andrea Gallo <andrea.gallo@stericsson.com>
> ---
> lib_generic/string.c | 19 +++++++++++++++----
> 1 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/lib_generic/string.c b/lib_generic/string.c
> index 181eda6..9bee79b 100644
> --- a/lib_generic/string.c
> +++ b/lib_generic/string.c
> @@ -446,12 +446,23 @@ char * bcopy(const char * src, char * dest, int 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;
> -
> + unsigned long *dl = (unsigned long *)dest, *sl = (unsigned long *)src;
>

Nitpick: Are you sure the casts are necessary here ?
I am not sure about U-Boot but Linux kernel maintainers frown on
unnecessary casts.

> + char *d8, *s8;
> +
> + /* while all data is aligned (common case), copy a word at a time */
> + if ( (((ulong)dest | (ulong)src | count) & (sizeof(*dl) - 1)) == 0) {
>

+ if ( (((ulong)dest | (ulong)src) & (sizeof(*dl) - 1)) == 0) {

The "or" should not include count: the remaining count % sizeof(unsigned
long) bytes are copied below.

> + while (count >= sizeof(*dl)) {
> + *dl++ = *sl++;
> + count -= sizeof(*dl);
> + }
> + }
> + /* copy the reset one byte at a time */
>

Nitpick: s/reset/rest/

> + d8 = (char *)dl;
> + s8 = (char *)sl;
> while (count--)
> - *tmp++ = *s++;
> + *d8++ = *s8++;
>
> return dest;
> }
>

Cheers,
Chris