Write a function to dynamically create a new copy of string and return a pointer to the new copy (C)

Question

this is what I have so far but I can't figure out what is wrong with it

void newCopy(char *s)
{
  char newString = malloc(sizeof(int * strlen(s)));
  newString = s;
  return &newString;
}

Answer 1

void newCopy(char *s)
{
  char newString = malloc(sizeof(int * strlen(s)));

First and second problems are here.

First is, You're assigning the return of malloc, which is a pointer, to a variable declared as char. The variable should be declared as char*.

Second is, your input to sizeof is wrong.

• int * strlen(s) is nonsense and won't compile, because you're trying to multiply a type and an integer. You meant sizeof(int) (which is an integer) * strlen(s) (also an integer) which will compile.
• You should use sizeof(char) instead of sizeof(int), since it is a string.
• You should add 1 to the size, since strings in C need to be null terminated by an extra \0 that strlen does not report being part of the string length.

Putting it all together, sizeof(char)*(strlen(s)+1)

  newString = s;

Third problem is here. = is not a magic operator - it assigns the value in the variable s (which is a pointer) to the value in the variable newString (which after fixing the above mistake, will also be a pointer). It does nothing beside that.

What you want to do instead is use strcpy, which is a function that copies the contents of one string (by following its pointer) to the contents of another string (by following its pointer). http://www.cplusplus.com/reference/cstring/strcpy/

  return &newString;

Fourth and fifth problems are here.

Fourth is, You have declared the function as void and here you are trying to return a char*.

Fifth is, you are trying to return a pointer to something that was declared on the stack (a local variable). As soon as the function returns, anything on the stack for that function is trash and cannot be referenced anymore.

However, if you correctly make newString of type char*, all you need to do is return newString; And you correctly return a pointer by value which points into the heap (thanks to the earlier malloc).

}

Finally, judging by this code, I should inform you that C is not a newbie friendly language, where you can just type things that 'look like' what you want to happen and pray it works. If you're even slightly wrong your code will crash and you will have zero idea why, because you don't know the right way to do things. Either read a really good book on C and teach yourself everything from basic to advance step by step so you know how it all works, or pick up a more user friendly language.

Answer 2

I should start by pointing out that in my opinion, given the number and (especially) nature of the mistakes in this code, you probably need to get a good book on C.

Your newString = s; overwrites the pointer instead of copying the string into the space you just allocated. Thus, you lose the pointer to what you just allocating (leaking the memory) without making a copy. You probably want to use strcpy instead of direct assignment.

Your computation of the size you allocate isn't what you really want either. Typically, for a string of length N, you want to allocate N+1 bytes. You're currently attempting to allocat sizeof(int * strlen(s)) bytes, which shouldn't even compile.

Answer 3

A corrected version should be like:

char *newCopy(char *s)
{
  if (s == NULL)
    return NULL;
  char *newString = malloc(strlen(s) + 1);
  if (newString == NULL)
    return NULL;
  strcpy(newString, s);
  return newString;
}