2

I have a noob question here, cannot understand whats wrong here. I have the following code here:

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


void copyFrom(char * mStr, char * str); 


int main() {  

    char * srcStr = "Robert bought a good ford car";  
    char * dstStr = NULL;  
    copyFrom(srcStr, dstStr);
    printf("The string copied here is %s", dstStr);
    return 0;
}
void copyFrom(char * str, char * mStr)
{
    if(!mStr) {
        mStr = (char *)malloc((strlen(str)) + 1);
        if(mStr == NULL)
            return;
    }

    while(*mStr++ = *str++) {
        ;
    }

    mStr[strlen(str)] = '\0';
}

This does not copy the string, But if array is used instead of char pointer for dstStr, it all works fine.

Can you please tell me what is wrong here?

KillianDS
  • 16,936
  • 4
  • 61
  • 70
Manohar
  • 1,270
  • 7
  • 17
  • 28
  • 3
    You _are_ copying the string. The problem is that `mStr` is reassigned to point to a local value.It being itself passed by value, this has no effect on `dstStr` in the caller. If you pass it as `char*& mStr` it should work. – Keith Aug 22 '13 at 06:10
  • @David schwartz, its a C++ code. – Manohar Aug 22 '13 at 06:18
  • Because you change pointer `dstStr`, an alternative method can be use of read this: [Pointer to pointer](http://stackoverflow.com/questions/18306935/need-of-pointer-to-pointer/18307020#18307020) – Grijesh Chauhan Aug 22 '13 at 11:22
  • Also you could change your function to ` void copyFrom(char * mStr, char ** str);` and call it with `copyFrom(srcStr, &dstStr);` – Alex G Aug 22 '13 at 06:16

6 Answers6

7

You need to return the value of mStr after you malloc it. Right now the return value of malloc and the pointer to your copy of the string disappears when you exit the function. It leaks the memory from malloc because it loses track of it.

Change your function to char* copyFrom(const char * str)

and then return mStr from that function. Then you can write dstStr = copyFrom(srcStr)

Now, POSIX C already has this function. It is called strdup.

Zan Lynx
  • 53,022
  • 10
  • 79
  • 131
3

In C, even pointers are passed by value. When you pass a pointer to a function, the function receives a copy--i.e., a new pointer that points to the same place as the pointer in the calling function.

So in your code, when you pass dstStr, you are passing a null pointer. mStr is a new null pointer. While you allocate memory for mStr, this memory is allocated to this new pointer, not to dstStr. So when your function returns, dstStr in the calling function is still NULL, and there is nothing pointing to the memory that was allocated in copyFrom(). You have a memory leak!

You need to pass dststr() as a two-level pointer in order to change what it points to in the calling function:

void copyFrom(char * str, char **dstStr)
{   
    char* mStr; 

    mStr = (char *)malloc((strlen(str)) + 1);
    if(mStr == NULL)
        return;


    while(*mStr++ = *str++) {
        ;
    }

    mStr[strlen(str)] = '\0';

    *dststr = mStr;

    return;
}

And of course your function declaration and call need to be changed accordingly. Your call would need to pass &dstStr as the argument, not just dstStr.

verbose
  • 7,827
  • 1
  • 25
  • 40
2
char * dstStr = NULL;
copyFrom(srcStr, dstStr);
printf("The string copied here is %s", dstStr);

dstStr is NULL, the first line set it. While copyFrom allocates a string, it doesn't return that string to the caller. So your printf gets a NULL.

David Schwartz
  • 179,497
  • 17
  • 214
  • 278
  • nope, problem is the fact he malloc withotu returnign th emalloc'ed data see @ZanLynx answer – Joel Falcou Aug 22 '13 at 06:13
  • @JoelFalcou: ZanLynx is merely *one* possible solution, among many. Were `dstStr` passed by reference (or pointer to pointer) it would also solve the issue. – Matthieu M. Aug 22 '13 at 07:06
  • If we had to dabble into reference, and then corect C++, then the OP betetr use std::string ;) – Joel Falcou Aug 22 '13 at 08:49
1

You can do this,

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

void copyFrom(char * mStr, char * str);

int main() {
    char * srcStr = "Robert bought a good ford car";
    char * dstStr = NULL;
    dstStr = (char *)malloc((strlen(srcStr)) + 1);
    copyFrom(srcStr, dstStr);
    printf("The string copied here is %s", dstStr);
    return 0;
}

void copyFrom(char * str, char * mStr)
{
    while(*mStr++ = *str++) {
        ;
    }

    mStr[strlen(str)] = '\0';
}
zahirdhada
  • 405
  • 4
  • 14
0

dstStr is not allocated so you're not puttign your data anywhere. Use malloc to allocate a proper memory block then copy.

The reason it works with array is that the string is stored on the stack with memory automatically allcoated for it.

Also this is C not C++ as C++ string handlign is done via std::string.

Joel Falcou
  • 6,247
  • 1
  • 17
  • 34
0

Rather than return, you can do pass the dest by reference:

void copyFrom(const char * src, char *& dest); 


int main() {  

    char * srcStr = "Robert bought a good ford car";  
    char * dstStr = 0;  
    copyFrom(srcStr, dstStr);
    printf("The string copied here is %s", dstStr);

    return 0;
}
void copyFrom(char * str, char *& mStr)
{
    if(!mStr) {
        mStr = (char *)malloc((strlen(str)) + 1);
        if(mStr == NULL)
            return;
    }
    char* it = mStr;
    while(*it++ = *str++)
    {
        ;
    }
    *it = 0;
}
Keith
  • 6,756
  • 19
  • 23