C: Missing some logic with the pointers stuff

Question

I am writing my own string copy function. The following works:

char *src, *dest;
src = (char *) malloc(BUFFSIZE);
//Do something to fill the src
dest = (char *) malloc(strlen(src) + 1);
mystringcpy(src, dest);

void mystringcopy(char *src, char *dest) {
   for(; (*dest = *src) != '\0'; ++src, +dest);
}

But this doesn't work:

char *src, *dest;
src = (char *) malloc(BUFFSIZE);
//Do something to fill the src
mystringcpy(src, strlen(src), dest);

void mystringcopy(char *src, size_t length, char *dest) {
   dest = (char *)malloc(length + 1);
   for(; (*dest = *src) != '\0'; ++src, +dest);
}

and I can't understand why... Is allocating memory inside a called function a mistake?

Answer 1

You haven't really said what "works" means, but I'm assuming you're confused why dest isn't being changed to the new memory back in the calling function.

The reason is that in your mystringcopy function, the parameter dest is a copy of the pointer dest in the calling function.

You then assign that copy to a new buffer, do the copy, and then the copy goes away. The original is unchanged. You need to pass dest as a pointer (to a pointer).

Also, I assume you wrote what you did from memory since it shouldn't compile as is (bad dereference in the calling function). Here's the fixed code:

char *src, *dest;
src = (char *)malloc(BUFFSIZE); // no dereference on src, it's a pointer

//Do something to fill the src
mystringcpy(src, strlen(src), &dest); // pass the address of dest

// take a pointer to a char*
void mystringcopy(char *src, size_t length, char **dest) {
    // now you should dereference dest, to assign to
    // the char* that was passed in
    *dest = (char *)malloc(length + 1);

    // for simplicity, make an auxiliary dest
    char* destAux = *dest;

    // and now the code is the same
    for(; (*destAux = *src) != '\0'; ++src, ++destAux);
}

Another method is to return the dest pointer:

char *src, *dest;
src = (char *)malloc(BUFFSIZE);

//Do something to fill the src
dest = mystringcpy(src, strlen(src)); // assign dest

char* mystringcopy(char *src, size_t length) {
    char* dest = (char *)malloc(length + 1);

    // for simplicity, make an auxiliary dest
    char* destAux = dest;

    for(; (*destAux = *src) != '\0'; ++src, ++destAux);

    return dest; // give it back
}

Keep in mind if length is smaller than the source buffer's real length that you'll overrun your destination buffer. See the comments for a solution, though this is left up to you.

Answer 2

Doing the malloc inside the function is OK, but you're not passing the pointer back out of the function. Either return the pointer:

char * mystringcopy(char *src)

or pass a pointer to the pointer:

void mystringcopy(char *src, char **dest)

Answer 3

Parameters in C are passed by value, so your function gets a copy of the dest pointer, overwrites it with malloc then discards it. Try this instead:

void mystringcopy(char *src, size_t length, char **dest) {
   *dest = (char *)malloc(length + 1);
   char *p=*dest;
   for(; (*p = *src) != '\0'; ++src, ++p);
}

Now you pass a pointer to the pointer to your string, so you can overwrite it in the main procedure. You'd use it like:

char *src, *dest;
*src = (char *) malloc(BUFFSIZE);
//Do something to fill the src
mystringcpy(src, strlen(src), &dest);
// now in dest you have your copy

Answer 4

There is no problem in allocating inside a function.

The problem is that in C arguments are passed by value. So when you assign a value to dest, that's only modifying the dest local to the function.

You have two choices. You can return the dest pointer:

char *alloc_and_copy(const char *src, size_t length)
{
    char *dest = malloc(length + 1);
    ... do your copying
    return dest;
}

or you can pass a pointer to the argument and modify what's being pointed to:

void alloc_and_copy(const char *src, size_t length, char **dest)
{
    char *local_dest = malloc(length + 1);
    ... do your copying using local_dest

    *dest = local_dest;
}

The technique of using a local variable is not required, but I think it makes for more readable code.

Answer 5

In general, when allocating memory there are certain assumptions about what code is responsible for freeing the memory when it's done. I subscribe to the notion that a function should be responsible for one major operation, as if a black box. For both reasons, it is best to allocate your own memory and hand the pointer to the function to populate its buffer.

That aside, you could either return the char * pointer as a return value.

Or, change the char *dest parameter to char **dest. Then, call the function like: mystringcopy(src, strlen(src), *dest). And in the function, it returns the pointer by: *dest = (char *)malloc(length + 1);. Not pretty.