My own function for copying memory in C

Question

I have written a function that copies memory from one destination to another. I would like you to view my code and tell me if I have to fix something. That's the code:

void* memCopy(void* destination, const void* source, size_t size)
{
 double *a = (double*)source, *b = (double*)destination;
 void* end = a + (size / sizeof(double));
 while ((void*)a < end)
 {
 *b = *a;
 ++a;
 ++b;
 }
 char* a2 = (char*)a;
 char* b2 = (char*)b;
 end = a2 + (size % sizeof(double));
 while (a2 < end)
 {
 *b2 = *a2;
 ++a2;
 ++b2;
 }
 return destination;
}

I have used double* to make the function faster.

• Mast: I'm not a fan of while loops for anything other than (near-)infinite loops. Is there a reason you used them instead of for loops? For reference, this is how one version of memcpy does it.
• Reinderien: @Mast agree-ish; but not because I have a specific aversion to while; rather because for is specifically a better fit here since it has a clear loop counter
• Mark Bluemel: The K&R approach would probably be "while((void *)a < end) { *b++ = *a++;}" ;-)
• Neil: If you look at the source for many x86 versions, you'll see that it 'top and tails' the copies on alignment boundaries, and then uses larger moves in the middle. Similar to how you are handling the end case.

Answer 1

Your code doesn't compile cleanly with GCC:

move.c: In function 'memCopy':
move.c:16:15: warning: comparison of distinct pointer types lacks a cast
 while (a2 < end)
 ^

Also 2 brief points:

1. Why reinvent the wheel? Most implementations of memcpy, memmove etc in the standard libraries will be correctly and efficiently implemented, in many cases not using pure C.
2. Your code assumes that both source and data are correctly aligned to be accessed as double. If they are not, depending on the platform the code may a) perform badly, b) give incorrect results or c) simply crash, unless I'm much mistaken. It happens to run (apparently correctly) on my PC, but that's no guarantee of portability.

The FreeBSD implementation shows how this can be done, assuming that that the mapping of pointers to uintptr_t gives values which can be masked appropriately - this is true of the relevant targets of the code, but may not be universally so.

• Toby Speight: Yes, exactly. I'd guess that Ivan tested on some form of x86, which is very forgiving about unaligned access. Most processors will bus fault at best.
• chux: Mark, Hmm, FreeBSD uses if ((unsigned long)dst < (unsigned long)src) {. I'd expect if ((uintptr_t)dst < (uintptr_t)src) {, that it is a bug when uintptr_t is wider than unsigned long.
• Mark Bluemel: @chux-ReinstateMonica I doubt that uintptr_t was even a twinkle in a C developer's eye when that code was developed - it's pretty ancient. I suspect that until it fails, it'll be left as is. As your answer mentions, it does deal with overlaps by appropriate directional copying. I didn't comment on the use of double, as you do, as I wasn't totally sure of my ground, but I agree with your concerns.
• chux: @MarkBluemel My recommendation does not introduce uintptr_t usage as the linked code already uses uintptr_t in if ((t | (uintptr_t)dst) & wmask) {. Unfortunately it does not use uintptr_t uniformly.

Answer 2

In addition to @Mark Bluemel good answer:

Overlap

A difference between memcpy() and memmove() is the ability to handle overlapping buffers. Notice the keyword restrict.

void *memcpy(void * restrict s1, const void * restrict s2, size_t n);
void *memmove(void *s1, const void *s2, size_t n);

restrict roughly implies access to the buffer is not interfered with by other activity.

memCopy() code does not copy well overlapping buffers when the source precedes the destination. It is more like

void* memCopy(void* restrict destination, const void* restrict source, size_t size)

By using restrict, the compiler can invoke additional optimizations.

A solution to act like memmove() involves looping up or down depending on which is greater source or destination. The trick is that this compare for pointer greatness is not certainly possible in C. memmove(), as a library function, has access to information C does not. Yet a compare of pointers converted to an integer is usually sufficient.

Types

double is not the way to go. *b = *a; is not guaranteed to copy the bit pattern nor potentially triggering a signaling not-a-number exception. Best to avoid involving floating-point concerns. Instead use a wide unsigned integer type, perhaps uintmax_t, unsigned long long, or uint64_t. (Pros and cons for each. I'd go for uint64_t when available.)

(削除) On ancient machines, *b2 = *a2; can invoke a trap. Instead use unsigned char *a2; unsigned char *b2; (削除ここまで)

Zero size

memCopy(..., ..., 0) is properly handled. Bravo! - a common mistake avoided here.