Deferencing Void Pointer / Void Pointer Copy

Question

• Platform: Linux 3.2.0 x86 (Debian Wheezy)
• Compiler: GCC 4.7.2 (Debian 4.7.2-5)

I am writing a function that copies the contents of a buffer to another buffer. I use a void pointer so that the function is not type specific. I have a testable version and it appears that the function is working properly. But I do not know if what I am doing is legal so my question is what are the pitfalls of what I have done if there are any.

#include <stdio.h>
#include <stdlib.h>

void* voidcpy(void *void_ptr, size_t nbytes)
{
    char *char_ptr = void_ptr;
    char *cpy_char_ptr = NULL;
    size_t i = 0;

    if((cpy_char_ptr = malloc(nbytes)) == NULL) return NULL;

    for(; i < nbytes; i++) cpy_char_ptr[i] = char_ptr[i];

    return cpy_char_ptr;
}

int main()
{    
    short int *intp = NULL;
    short int *cpy_intp = NULL;
    size_t siz = 5;
    int i = 0;

    if((intp = malloc(siz * sizeof(short int))) == NULL)
    {
        perror("(malloc)");
        return -1;
    }

    intp[0] = 0;
    intp[1] = 14;
    intp[2] = 187;
    intp[3] = 12678;
    intp[4] = -234;

    if((cpy_intp = voidcpy(intp, siz * sizeof(short int))) == NULL)
        return -2;

    printf("intp = %p\ncpy_intp = %p\n\n", (void*)intp, (void*)cpy_intp);

    for(; i < siz; i++) printf("cpy_intp = %i\n", cpy_intp[i]);

    free(intp);
    free(cpy_intp);

    return 0;
}

Answer 1

Yes, this is perfectly legal, in C you can legally assign a void pointer to any other pointer type and you can assign any pointer type to a void pointer.

In C++ this is not allowed. In C++ you would have to use a reinterpret_cast to cast to a different pointer type because the free void pointer casting that is allowed in C is considered a "loop hole" that is easy to make mistakes with.

Of course there is a truth to that idea, if you're not careful you could be doing the wrong thing, e.g. you could easily pass a pointer to a pointer to this function by mistake and then your function will happily overwrite whatever is on the stack beyond that pointer. This is no fault of your implementation however, it's just the way the function is to be used and memcpy behaves no differently.

Nevertheless you would be better off using memcpy instead as this will very likely be much better optimized, though these days the compiler will probably make a pretty decent version out of your code as well.

A few more pointers;

1) you do not need to malloc the original array, you can initialize it statically like so

short int int_arr[] = {
    0,
    14,
    187,
    12678,
    -234,
};

2) you can then call your function in the following way:

cpy_int_arr = voidcpy(int_arr, sizeof(int_arr));

3) if you don't want to define the array statically, then use the pointer to get the element size, that way you can change the array type without needing to change it somewhere else in the code, which reduces the potential dangers of the "loop hole" void casting:

cpy_intp = voidcpy(intp, siz * sizeof(*intp));

4) you don't need to cast to void* in the printf call

5) try to assign variables immediately and do not put assignments inside of if statements:

char *cpy_char_ptr = malloc(nbytes)
if (cpy_char_ptr == NULL)
    return NULL;

6) similarly you can define a iteration variable inside the loop clause:

for(size_t i = 0; i < nbytes; i++) cpy_char_ptr[i] = char_ptr[i];

The reason to define variables as late as possible and initialize them immediately is that you keep the scope of the variables as small as possible and you can not mistakenly use a variable before it is initialized.

7) (personal preference) don't use type names in your identifiers (intp, voidcpy) your code will either become difficult to read/understand if your identifier states a type different from what the variable actually is, (e.g. your type is actually a short int and not an int as the variable name would suggest,) or you will need to change the identifier throughout the entire code whenever you change the type with the possibility of making mistakes.