0

Hello I am trying to copy a char * pointer to a char [] array.

this is my code so far

char * string_add(char * base, char * toAdd)
{
    char * string=malloc(strlen(base)+streln(toAdd)+1);
    sprintf(string,"%s%s",base,toAdd);
    char returnString[strlen(string)+1]; 
    // here comes my problem:
    memcpy(returnString,string,strlen(string)+1);
    // want to add free(string) here
    return returnString;
}

I want to use such a function to save code. I don't want to look after every allocated memory. I also tried

memcpy(&returnString,string,strlen(string)+1);

and some variants with strcpy and strncpy. But following problem persists:

if I call the function twice like:

int main(int argc, char * argv[])
{
    char * str1=string_add(argv[1],"-test1"); 
    char * str2=string_add(argv[1],"-test2"); 
    printf("%s, %s", str1,str2);
}

the output is like:

abc-test2, abc-test2

How can I realize this?

Thank you in advance!

SevenOfNine
  • 630
  • 1
  • 6
  • 25
  • 2
    You're returning the address of an automatic variable `returnString`. You shouldn't need more than the `string` buffer. Just set `string[strlen(base) + strlen(toAdd)] = 0` (or use the return value of `sprintf` to know how long `string` is, or save `strlen(base) + strlen(toAdd)` before you use `malloc`) and `return string`. – Seth Carnegie Aug 18 '13 at 21:23
  • but if I `return string` I have to free the result later. This is what i want to avoid. – SevenOfNine Aug 18 '13 at 21:30
  • you can't really avoid it. if you have the option of C++, then you can use smart pointers, but in C, you either have to preallocate, or you manage your malloced memory – Keith Nicholas Aug 18 '13 at 21:36
  • @user2430568 even if you don't `return string` you still have to free it. As it is now, your code has a memory leak. – Seth Carnegie Aug 18 '13 at 21:38

5 Answers5

3

in C, you have to look after the malloced memory, the char array you are declaring is on the stack, and will be gone after the function returns, only the malloc memory will hang around. and Yes, you will have to look after it and clean it up.

Keith Nicholas
  • 43,549
  • 15
  • 93
  • 156
  • yes but I am trying to create this function so I don't have to look after every string i am creating. I want it like: Call the and get an array[] with everything cleaned up already – SevenOfNine Aug 18 '13 at 21:27
  • you can't do what you want very easily ( and possibly not at all depending on your application ). Your issue is that a char[] only live within the scope of the function (unless its global). – Keith Nicholas Aug 18 '13 at 21:32
  • is it bad style to make a var global if I need it in every function? – SevenOfNine Aug 18 '13 at 21:34
0

I assume that the second call to your function overwrites the contents of the buffer created by the first call.

To my understanding, you are trying to concatenate two character strings.

Have a look in this answer.

Basically, since the length of the strings is not be known at compile-time, you'll need to allocate dynamic memory, while not forgetting to free it after you are done handling the new string.

Community
  • 1
  • 1
Novak
  • 2,760
  • 9
  • 42
  • 63
0

In the function

char * string_add(char * base, char * toAdd)
{
    char * string=malloc(strlen(base)+streln(toAdd)+1);
    sprintf(string,"%s%s",base,toAdd);
    char returnString[strlen(string)+1]; 
    // here comes my problem:
    memcpy(returnString,string,strlen(string)+1);
    // want to add free(string) here
    return returnString;
}

You define returnString[] but it only lives within the scope of the function. So you have to change the logic behind the use and the declaration of returnString[]

Oscerd
  • 1,616
  • 11
  • 14
  • yes I agree. If i do this for example `char * test="blabla"` i won't have to call `free(test)` i want to return a char which i dont have to free! – SevenOfNine Aug 18 '13 at 21:37
0

Basically, storage that is NOT malloc'ed does NOT persist after a routine ends. You might be able to use global vars, but that would complexify a simple task, i.e. concatenate two strings.

char * string_add(char * base, char * toAdd)
{
    char *string_ptr = malloc(strlen(base)+streln(toAdd)+1);

    sprintf(string_ptr, "%s%s", base, toAdd);

    // string_ptr points to heap storage that is permanent
    return string_ptr;
}

// in main() ....

char *s1 = string_add("A", "B");
// s1 points to storage on the heap as: "AB\0"
//
free(s1);  // OR allow system to clean-up when main() exits
JackCColeman
  • 3,777
  • 1
  • 15
  • 21
0

So there's a bit wrong with that code. First off you do:

char * string=malloc(strlen(base)+streln(toAdd)+1);

that is wrong, that assumes every char is 1 byte which it depends on the platform. What's better to do is:

char * string=malloc((strlen(base)+streln(toAdd)+1) * sizeof(char));

plus malloc is expensive in terms of CPU time and you don't free it. Doing what you're doing in that code doesn't need the malloc even. Just do this:

char returnString[strlen(base) + strlen(toAdd) + 1];

second of all, you should deal with the strings differently. What you can do is this:

strcpy(returnString,base);
strcat(returnString,toAdd);

or. The best thing to do for something like this is declare in your main str1 and str2, malloc them in main, pass the pointer to the pointer to the function. Have the function copy/cat the data like i showed above. and then finish. Now you don't need to deal with return types and all that inner mallocing and crap like that. You just deal with two mallocs in main and 2 free's in main after you use them. Here's the pseudo code:

int main(int argc, char *argv[])
{
    char * str1 = (char *) malloc((sizeof(argv[1]) + sizeof("-test1")) * sizeof(char));
    //do same for str2
    copyString(&str1,argv[1],"-test1");
    //do same for str2
    //print
    //free like this:
    free(str1);
    //do same for str2
    return 0;
}

now here's the copyString function:

void copyString(char ** returned, char * base, char * add)
{
    strcpy(returned,base);
    strcat(returned,add);
} 
Ion
  • 334
  • 3
  • 15
  • 1
    char is defined to be 1 byte wide by the standard, but even if it weren't sizeof is defined in terms of char, not byte width. `sizeof(char)` is guaranteed to be 1. – tab Aug 18 '13 at 23:07
  • Yes it's always going to be 1, but it is good practice to get into the habit of doing that. Since on different systems the sizeof(int) or sizeof(double) for example could vary. It also makes code more readable. – Ion Aug 18 '13 at 23:35