1

I am having trouble trying to implement a custom strcpy function which is supposed to handle cases where the src string is larger than the destination string. Here I have provided some code so that you guys can see the entire function. My issue is that every time I increment *dest, it goes into a null address despite the fact that I have allocated enough memory to fit all of src in it. This causes the a segmentation fault in (double pointer)dest = *src. dest is stored as a char** because in reality, the argument that has to be passed is another string that is possibly of a smaller size than src, and I wish to overwrite *dest as safely as I can.

int customStrCpy(char** dest, char* src){
    int strlen1 = strlen(*dest), strlen2 = strlen(src);
    if(strlen1 < strlen2){
        //Creates a dynamically allocated array that is big enough to    store the contents of line2.
        *dest = calloc(strlen2, sizeof(char));
        char* backup_str = *dest;

        int copy_arrs;
        for(copy_arrs = 0; copy_arrs < strlen2; copy_arrs++){
            **dest = *src;
            *dest++; src++;
        }
        *dest = backup_str;
    }
    else strcpy(*dest, src);
}

In the end, (char**)dest is supposed to be pointing to the correct string.

Barmar
  • 741,623
  • 53
  • 500
  • 612
  • `strlen` is returning number one less than you want. Mind the terminating `\0`. Also - is `dest` always pointing to a valid string when passed to this function? – Eugene Sh. Apr 25 '19 at 18:50
  • 1
    `strlen(*dest)` will only tell you the current number of characters in *dest. If it has been allocated prior to the call of your method, you have no way of knowing how much space has been allocated without also passing that length into your method. – FredK Apr 25 '19 at 18:54
  • You do realize that this function, even after you fix it, is basically a memory leak factory? – Lee Daniel Crocker Apr 25 '19 at 18:55
  • If you replace the destination string with a newly allocated string, you should also free the original allocation. – Barmar Apr 25 '19 at 18:56
  • 1
    Why not use `strcpy()` after `calloc()`? Also, why use `calloc()` instead of `malloc()`, if you're going to immediately overwrite it? – Barmar Apr 25 '19 at 19:00
  • You just reimplemented [the POSIX standard function `strdup()`](https://pubs.opengroup.org/onlinepubs/9699919799/functions/strdup.html). – Andrew Henle Apr 25 '19 at 20:06
  • @AndrewHenle No he didn't. `strdup()` won't reuse an existing string if it's big enough. – Barmar Apr 25 '19 at 20:14
  • @Barmar OK, he created a dangerous `strdup()` that might or might not leak memory. As implied earlier... – Andrew Henle Apr 25 '19 at 20:33
  • @AndrewHenle True. My answer shows how to prevent the leak. – Barmar Apr 25 '19 at 20:36

3 Answers3

0

You need to add 1 to the string length, to allow for the null terminator, and you should free the old contents of dest if you're allocating a new string. After you do this, you can do the same strcpy() as you do when you don't need to reallocate.

There's also no need for the int return type (unless you want to add error checking to malloc(), and return a status result). This function modifies an argument, it should be void.

void customStrCpy(char** dest, char* src){
    int strlen1 = strlen(*dest), strlen2 = strlen(src);
    if(strlen1 < strlen2){
        free(*dest); // Free the old string
        //Creates a dynamically allocated array that is big enough to store the contents of line2.
        *dest = malloc(strlen2+1);
    }
    strcpy(*dest, src); // or memcpy(*dest, src, strlen2+1);
}
Barmar
  • 741,623
  • 53
  • 500
  • 612
  • Now we just have to return something useful… also, `strcpy()` when you *know* the length? – Deduplicator Apr 25 '19 at 20:40
  • There's little gained from using `strncpy()`, since you know it will fit. Could use `memcpy()`. I'm not going to worry about that. – Barmar Apr 25 '19 at 20:45
  • `void ` or `int` is illogical. using strlen to determine if there is enough space also - the destination string might be changed many times before the call. – 0___________ Apr 25 '19 at 20:54
  • @P__J__ I'm not questioning the basic premise of the question. – Barmar Apr 25 '19 at 20:56
  • @Barmar I have tried to use memcpy, strcpy, strncpy, etc... all of them have completely screwed up the strings when trying to copy larger strings into smaller ones, both the destination and source strings. – NoobProgrammer Apr 25 '19 at 21:06
  • @NoobProgrammer Of course. You still have to allocate a new string first if the destination isn't large enough. – Barmar Apr 25 '19 at 21:11
  • @NoobProgrammer The basic problem with your question is that you assume that `strlen(*dest)` is the amount of memory allocated for the destination. It could be larger, but there's no way to know with `strlen()`, so you might reallocate when it's not really necessary. It would be better to pass the destination buffer's size as a separate parameter. – Barmar Apr 25 '19 at 21:14
  • There is never anything to be gained from using `strncpy()`, unless you don't actually want to write a string. – Deduplicator Apr 25 '19 at 21:17
  • @Deduplicator I just assumed that's what you were alluding to when you mentioned knowing the length. – Barmar Apr 25 '19 at 21:24
  • I was alluding to `memcpy()`, which you mentioned at the end of that note. I didn't consider `strncpy()` as a viable alternative at all as I don't see it as a string-function, though strictly speaking, with a known length, it happens that one can cludge it in there. – Deduplicator Apr 25 '19 at 21:30
0

Usually strcpy returns char * for "direct" use in other operations.

char *mysStrCpy(char **dest, const char *src)
{
    size_t len = strlen(src);
    char *tmpptr;

    *dest = malloc(len + 1);
    // or *dest = realloc(*dest, len + 1);
    if(*dest)
    {
        tmpptr = *dest;
        while(*tmpptr++ = *src++);
    }
    return *dest;
}
0___________
  • 60,014
  • 4
  • 34
  • 74
-1
 *dest++;

increments dest, not the pointer dest points to. You want:

(*dest)++;

ps: there are better ways to accomplish what you are after....

mevets
  • 10,070
  • 1
  • 21
  • 33
  • If you increment the pointer that `dest` points to, then the caller won't get a pointer to the beginning of the returned string, they'll get a pointer to the end of it. – Barmar Apr 25 '19 at 18:58
  • The right solution is to use a local variable for this, not the caller's pointer. Or just call `strcpy()`. – Barmar Apr 25 '19 at 18:58
  • He seemed to want it to point at the end of the string; but yes in the comment he says something different. – mevets Apr 25 '19 at 20:29
  • actually, no, you are wrong. that is what backup_str does. – mevets Apr 25 '19 at 20:30
  • Oops, didn't notice that. If you edit your answer, I can unvote. – Barmar Apr 25 '19 at 20:31