0

I am trying to create a char* strcpy(char* dest, const char*) function. This is what I have so far:

char* mystrcpy(char *dest, const char *src)
{
    char* tempsrc = (char*) src;
    int length = strlen(src);
    char tempdest[length];

    for(int i = 0; i < length;i++)
    {
        tempdest[i]= tempsrc[i];    
    }
    tempdest[length] = '\0';
    return dest = tempdest;
}

It returns the correct dest variable, so it can be saved in the main method, but if the return isn't saved, the *dest in the main method isn't changed like I would want it to.

How do I change the dest* pointer within strcpy?

[EDIT]

Here is the full method I have been using to test this

#include <stdio.h>
#include <string.h>

int main(int argc, char** argv){
char* source = "stringone";
char* dest = "stringtwo";
char* mystrcpy(char *dest, const char *src);

printf("Before src: %s\n", source);
printf("Before dest: %s\n", dest);
mystrcpy(dest, source);
printf("After src: %s\n", source);
printf("After dest: %s\n", dest);

}


char* mystrcpy(char *dest, const char *src)
{
    int length = strlen(src);

    for(int i = 0; i < length;i++)
    {
        printf("test\n");
        dest[i] = src[i];
        printf("test1\n");      
    }
    dest[length] = '\0';
    return dest;
}
rdennis42
  • 25
  • 6
  • Remember that arrays in C are zero based. An array of `length` elements have indexes from `0` to `length-1` (inclusive). Also, you do know that strings have an extra terminator character. However this is not counted by `strlen` though. – Some programmer dude Jan 20 '17 at 19:43
  • `char tempdest[length];` ==> `char tempdest[length+1];` – Weather Vane Jan 20 '17 at 19:44
  • 1
    Read up on pass-by-value and pass-by-reference. You are passing a copy of `dest` to the function. – 001 Jan 20 '17 at 19:44
  • Also, think about what happens to the array `tempdest` once the function returns. – Some programmer dude Jan 20 '17 at 19:44
  • You return a pointer to `char tempdest[length];` which is about to be forgotten. The required target was `dest`, no? – Weather Vane Jan 20 '17 at 19:45
  • C does not support _methods_. – too honest for this site Jan 20 '17 at 19:45
  • Have you tried a double pointer ? Remember that doing `return dest = tempdest;`, you're assigning the location of dest to tempdest, without changing the original pointer (it's a copy). – Alan CN Jan 20 '17 at 20:12
  • @JohnnyMopp I think this was my problem, I'm trying to pass by reference but I'm not sure how to do that, I thought since it was taking char *dest that would be pass by reference, why isn't it? – rdennis42 Jan 20 '17 at 20:22
  • @AlanCN I'm pretty new in C and I'm not totally sure, but I thought I was assigning the location of tempdest to dest, not the other way around. And that is what I would want, since dest would then point to the resulting char* – rdennis42 Jan 20 '17 at 20:24
  • @WeatherVane right, the required target was dest, which is why I tried to save it in dest in the last line. When I try to iterate through dest and src instead of tempdest and tempsrc, I get a segmentation fault. – rdennis42 Jan 20 '17 at 20:26
  • You did not "save" anything in `dest`, all you did was to change the local copy of a pointer supplied, which will also be forgotten. – Weather Vane Jan 20 '17 at 20:28
  • From the question update: you cannot write anything to `char* dest = "stringtwo";` because it is a read-only string literal. Try `char dest[1000] = "stringtwo";` The first creates a pointer to read-only memory, the second creates an array that you can write up to 999 characters to. – Weather Vane Jan 20 '17 at 20:31
  • @WeatherVane so if the function I need to implement is char* strcpy(char *s1, const char *s2), could I still pass in the dest if it's declared as an array instead of the string literal? I'm sorry if these seem like stupid questions, but I've really only worked with object oriented languages before so the whole pointer system in C is confusing for me – rdennis42 Jan 20 '17 at 20:35
  • @WeatherVane also is there a way to do it with dynamically allocated memory instead of limiting it at 999 characters or wasting all that space for a small string? – rdennis42 Jan 20 '17 at 20:38
  • I put `1000` as arbitrary large size, because `strcpy` is unsafe. That and memory allocation are other issues. Dynamic memory is outside the scope of this question: SO questions should not evolve into dynamic tutorials. – Weather Vane Jan 20 '17 at 20:42
  • @WeatherVane that's fine, thanks for your help I appreciate it – rdennis42 Jan 20 '17 at 20:43

2 Answers2

2

What you're doing is returning a pointer to a local variable, i.e. tempdest. This variable goes out of scope when you return from the function, so attempting to dereference that pointer invokes undefined behavior.

The function is being given a destination buffer to write to, so just write to it directly instead of a separate buffer.

char* mystrcpy(char *dest, const char *src)
{
    int length = strlen(src);

    for(int i = 0; i < length; i++)
    {
        dest[i] = src[i];    
    }
    dest[length] = '\0';
    return dest;
}

If on the other hand the idea is to create a new buffer and return that, you need to allocate memory dynamically via malloc. You would also need to pass a pointer-to-pointer for the first parameter so you can change the pointer value in the calling function.

char* mystrcpy(char **dest, const char *src)
{
    int length = strlen(src);
    *dest = malloc(length+1);
    if (*dest == NULL) {
        perror("malloc failed");
        exit(1); 
    }

    for(int i = 0; i < length; i++)
    {
        (*dest)[i] = src[i];    
    }
    (*dest)[length] = '\0';
    return *dest;
}

EDIT:

The reason you segfault with the first implementation is because you're not passing in a character array, but a pointer to a string literal. String literals typically live in a read-only area of memory, and attempting to write there is (again) undefined behavior.

Your destination buffer needs to be a writeable array big enough to store the string you want.

char* source = "stringone";
char dest[50] = "stringtwo";
dbush
  • 205,898
  • 23
  • 218
  • 273
  • It has to be `length+1`. – Amin Negm-Awad Jan 20 '17 at 19:56
  • @AminNegm-Awad Good catch. Edited. – dbush Jan 20 '17 at 19:57
  • This is what I had at first (the first one), and I get a segmentation fault at the line dest[i] = src[i]; – rdennis42 Jan 20 '17 at 20:18
  • @rdennis42 Then you must have passed in a pointer that didn't point to anything. It sounds as though the second code block is what you want. – dbush Jan 20 '17 at 20:21
  • @dbush I would like to change the original dest though, instead of allocating memory- so from your description I would think I want the first one. I edited the post to include the main function I'm using in case there is a problem there. – rdennis42 Jan 20 '17 at 20:31
  • @dbush my error with that was that I had dest declared as a read-only string literal, thanks for your help. Is there a way to do this more efficiently in terms of not using a large array when it's not needed, while still being able to deal with large strings? – rdennis42 Jan 20 '17 at 20:39
  • @rdennis42 Glad I could help. Feel free to [accept this answer](http://stackoverflow.com/help/accepted-answer) if you found it useful. – dbush Jan 20 '17 at 20:41
1

You change dest in your function, but the change is not propagated to the caller. This is, because in C arguments are passed to the function by value.

To get a by-reference style argument passing, you "simply" have to add another level of indirection. In your case that means: If you want to change the value of the pointer itself, you have to pass a pointer to the pointer, a pointer-pointer:

char* mystrcpy(char **dest, const char *src) // Another level of indirection

Inside the function you have to dereference it:

*dest = tempdest;

However, this will return the pointer, but it is invalid: *dest then points to what tempdest points to: a local var. It loses its extent when finishing the function, so on the caller's site it points to unowned memory. You have to use malloc() instead of local memory to allocate memory still available on the caller's site:

char *tempdest = malloc( length + 1); **

On the caller's side you have to add the indirection, too:

char *dest;
mystrcpy( &dest, src );

Of course, you have to free the memory explicitly, when you do not need it anymore:

free(dest);

** length is not enough, because strlen() does not count the \0 terminator. But you need memory for it. So you have to add 1.

Amin Negm-Awad
  • 16,582
  • 3
  • 35
  • 50
  • My only problem with this is that the function I need to implement is char* strcpy(char *s1, const char *s2), so I can't change the datatypes it takes – rdennis42 Jan 20 '17 at 20:32