Copy string in C dynamic

Question

I have two versions of C Codes. The first one works but the second one does not work.
The output of the second one is a segmentation fault but I don't know why.
Maybe someone can explain me my mistake? I would really appreciate that.

this works:

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

void stringcpy(const char*, char*);

    int main() {

        const char * original = "C is fun.";
        int size = sizeof(original) / sizeof(char*);
        copy = (char*)malloc(sizeof(char) * 11 + 1);

        stringcpy(original, copy);
        printf("%s\n", copy);

        free(copy);

        return 0;
    }

    void stringcpy(const char* original, char* copy) {

        int i;
        for(i = 0; *(original + i) != '\0'; i++) {
            *(copy + i) = *(original + i);
        }
        *(copy + i) = '\0';
    }

this doesn't work:

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

void stringcpy(const char*, char*);

int main() {

    const char * original = "C is fun.";

    char * copy;
    stringcpy(original, copy);
    printf("%s\n", copy);

    free(copy);

    return 0;
}

void stringcpy(const char* original, char* copy) {

    int size = sizeof(original) / sizeof(char*);
    copy = (char*)malloc(sizeof(char) * size + 1);

    int i;
    for(i = 0; *(original + i) != '\0'; i++) {
        *(copy + i) = *(original + i);
    }
    *(copy + i) = '\0';
}

Fyi, arguments in C are passed by value. Meaning: `copy = ...` in your function means nothing to the caller. Think about that. Unrelated [don't cast `malloc` in C programs](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). — WhozCraig, Feb 02 '18 at 16:44
The variable `original` is a pointer. Doing `sizeof` on a pointer give you the size of the *pointer* and not what it points to. If you want the length of a string, use [`strlen`](http://en.cppreference.com/w/c/string/byte/strlen). — Some programmer dude, Feb 02 '18 at 16:44
[Do I cast the result of malloc?](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — Barmar, Feb 02 '18 at 16:45
Also you use the `sizeof(array) / sizeof(array[0])` idiom wrong anyway. — Some programmer dude, Feb 02 '18 at 16:46
You have something equivalent to `sizeof(array) / sizeof(&array[0])`. Compare it to the example it my previous comment. — Some programmer dude, Feb 02 '18 at 18:12

user2736738 · Answer 1 · 2018-02-02T17:22:20.710

int size = sizeof(original) / sizeof(char*);

Applying sizeof to a pointer not an array over here. This will not return the size of the string to which it points to. strlen is the one you should look for here. Replace the above line with this (Solution to the size determination)

int size = strlen(original); // better to use `size_t`

Now here what is going on - the string literal is a char array which decays into pointer to the first element and that pointer value is being assigned - over which you applied sizeof. There is no trace of array here. To get the number of elements an array can hold you have to pass the array itself not the pointer.

Another thing is C is pass by value - so here to retain the changes either pass the pointer to the pointer or pass the allocated memory address from the function.

sizeof(char) is 1 so you don't need to write it explicitly - you can simply write

copy = malloc(size + 1);

Code:

stringcpy(original, &copy);

And

void stringcpy(const char* original, char** copy) {

    int size = strlen(original);
    *copy = malloc(size + 1);

    int i;
    for(i = 0; original[i] != '\0'; i++) {
        (*copy)[i] = original[i];
    }
    (*copy)[i] = '\0';
}

I have shown you the solution of the problem by passing the address of the pointer variable to the other function. Alternatively (try it yourself) you can return the allocated memory chunk's address from the function and assign it to the copy. This would also work.

Edit: If you are not allowed to change the function signature then yes it's not possible unless you provide the allocated memory to the copy like you did before in case-1.

The otherway to achieve correct behavior would be to do this

char* stringcpy(const char* original) {

    int size = strlen(original) ;
    char* copy = malloc(size + 1);

    int i;
    for(i = 0; original[i] != '\0'; i++) {
        copy[i] = original[i];
    }
    copy[i]=0;
    return copy;
}

And use it like this

copy = stringcpy(original);

Few things omitted in this answer:

Always check the return value of malloc - in case it returns NULL you won't dereference it if you put a check of it.
Don't cast the return type of malloc - char* to void* conversion is implicit.
Free the dynamically allocated memory when you are done working with it.

@AmirKhan.: Now check and read the aswer carefully....hope it helps. — user2736738, Feb 02 '18 at 17:17

score 0 · Answer 2 · answered Feb 02 '18 at 16:48

0

The memory that you allocate inside the function is allocated from the heap. When you exit the function, that memory is given back to the system, so that it is no longer allocated when you return.

answered Feb 02 '18 at 16:48

rallen911

148
9

The memory is *not* "given back to the system", it is *leaked*, which is arguably worse in-practice, but still problematic. Heap allocations are never refunded without programmatic intervention, save for the process actually terminating. – WhozCraig Feb 02 '18 at 16:51
So the only way to do that is the first code? I am not allowed to change the function parameters. – Amir Khan Feb 02 '18 at 16:51
@AmirKhan If you cannot change argument types, then the expectation *must* be that the supplied "destination" argument is already populated with an address referring to memory that is sufficiently large enough to accept the copy operation. I use "must" loosely here, as it is also conceivable either the assignment is poorly conveyed, or your interpretation is. They, too, are possibilities. – WhozCraig Feb 02 '18 at 16:54

Copy string in C dynamic

2 Answers2

Code: