2

I am writing a very simple program to copy a string using malloc.

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

char * copyStr(char s[])
    {
      int len = strlen(s); //find length of s
      char * copy; 
      copy = (char *)malloc(len); //dynamically allocate memory 
      int i; 
      for(i=0;i<len;i++) 
       {
         copy[i]=s[i];  //copy characters               
       }
       return copy; //return address
    }

int main(int argc, char ** argv)
    {
     char * str;
     str = "music is my aeroplane";
     char * res;
     res = copyStr(str);
     printf("The copied string is : %s",res);
     getch();
    }

The desired output is:

The copied string is : music is my aeroplane

The current output is:

The copied string is : music is my aeroplaneOMEeJ8«≤╝

Any advice is appreciated.

Arjun
  • 817
  • 3
  • 16
  • 28
  • 3
    Your copy is not a C string, because it is not null terminated. – M Oehm Apr 18 '16 at 12:42
  • 1
    [Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). – Sourav Ghosh Apr 18 '16 at 12:47
  • @SouravGhosh That's an interesting read, thanks. – Arjun Apr 18 '16 at 12:49
  • I love casting malloc. The misguided, lenient C type system is no reason to be sloppy. C++ tightened it wherever it could for a reason. – Peter - Reinstate Monica Apr 18 '16 at 13:06
  • To clarify what @PeterA.Schneider wrote: Every unnecessary cast inhibits type-checking by the compiler. Which in turn has potential to not detect a type error automatically. Thus the general rule only to use a cast iff 1) it is absolutely necessary, 2) you fully understand **all** implications, and 3) completely accept them. Said than, casting `void *` definitively **is** a bad idea. – too honest for this site Apr 18 '16 at 13:13

2 Answers2

7

A C string is null terminated. Add a null character (ie the char which ASCII code is 0) at end of the string :

char * copyStr(char s[])
{
  size_t len = strlen(s); //find length of s
  char * copy; 
  copy = (char *)malloc(len + 1); //dynamically allocate memory
                           /* One more char must be allocated for the null char */

  size_t i; 
  for(i=0;i<len;i++) 
   {
     copy[i]=s[i];  //copy characters               
   }
   copy[i] = '\0'; // HERE
   return copy; //return address
}

It is better to use size_t for the lengths because it is unsigned.

Boiethios
  • 38,438
  • 19
  • 134
  • 183
  • You could insofar it will compile, but don't. NULL is not a char (1 byte) but a pointer not set : `(void *)0` – Boiethios Apr 18 '16 at 12:50
  • 4
    @Arjun: A simple `0` would do as well. And please note that the ASCII character named `NUL` is something completely different then the null-pointer constant `NULL`. – alk Apr 18 '16 at 12:51
  • 2
    @Arjun No, it isn't. It would work for complex reasons that we don't need to discuss. But a NULL pointer and a null character are two different beasts that are confusingly named in the C standard. – Art Apr 18 '16 at 12:51
  • @Art Okay, thanks. I think size-wise we prefer '\0' right? 4 bytes vs 1 byte? – Arjun Apr 18 '16 at 12:58
  • @Art The C standard is very clear about this, the ASCII character's name with value 0 is called `NUL`, the pointer which compares true to 0 is called `NULL`. – hroptatyr Apr 18 '16 at 12:59
  • @hroptatyr "Very clear" would be different. I'm glad they don't have _NUL, __NULL, NIL or null. – Peter - Reinstate Monica Apr 18 '16 at 13:08
  • 1
    @hroptatyr Is it? I can't find the string "NUL " anywhere in the C11, C99 or C89 standards. "null character" is used all over the place though. – Art Apr 18 '16 at 13:09
  • @Art Sorry, my bad, you're right, the official final C11 standard does talk about "null characters" and "null wide characters", probably because ASCII isn't mandatory and `NUL` is an ASCII-specific name; I take everything back I said – hroptatyr Apr 18 '16 at 13:15
  • @Art Interestingly NUL appears in the n1570 draft in just one place, a footnote in 7.4.1.2. Afaics it is not defined in stdio.h; I always thought the canonical way to write the null char is `'\0'`. – Peter - Reinstate Monica Apr 18 '16 at 13:16
  • I prefer to use '\0' because it's clearer what it's actually supposed to mean. But 0 will always be correct too. – Art Apr 18 '16 at 13:19
  • @Arjun `'\0'` is an `int`. `0` is an `int` - Both have the same size. Both are suitable for _null character_ assignment. `NULL` is the _null pointer constant_. It is best used with pointers. Do not use when assigning the _null character_. – chux - Reinstate Monica Apr 18 '16 at 13:47
5

Strings in C are null-terminated.

The C programming language has a set of functions implementing operations on strings (character strings and byte strings) in its standard library. Various operations, such as copying, concatenation, tokenization and searching are supported. For character strings, the standard library uses the convention that strings are null-terminated: a string of n characters is represented as an array of n + 1 elements, the last of which is a "NUL" character.

In this case, you should have enough memory to store the original string contents and the "NUL" character (length+1). And don't forget to ensure the presence of the "NUL" character after the end of the string.

There are many possible ways to implement char * copyStr(char[]) function:

1) Your way (corrected):

char * copyStr(char s[])
{
    int i;
    size_t len = strlen( s );

    char * p = (char*) malloc( len + 1 );

    for( i = 0; i < len; i++ )
        p[i] = s[i];

    p[i] = '\0';

    return p;
}

2) Using memcpy():

char * copyStr(char s[])
{
    size_t len = strlen( s );

    char * p = (char*) malloc( len + 1 );

    memcpy( p, s, len );

    p[ len ] = '\0';

    return p;
}

3) Using strcpy():

char * copyStr(char s[])
{
    size_t len = strlen( s );
    char * p = (char*) malloc( len + 1 );
    strcpy( p, s );
    return p;
}

4) Using strdup():

char * copyStr(char s[])
{
    return strdup(s);
}

Note: For every malloc() function call you need a free() function call, your main() needs a little modification for correctness:

int main( int argc, char ** argv )
{
    char * str;
    char * res;

    str = "music is my aeroplane";

    res = copyStr( str );

    printf( "The copied string is : %s", res );

    free(res); /* freedom */

    getch();

    return 0;
}

Hope it Helps!

Lacobus
  • 1,590
  • 12
  • 20
  • Brilliant, I'm kind of lost for words. Yes, I understand that I should free the dynamically allocated memory. I forgot previously.. – Arjun Apr 18 '16 at 14:16
  • I see a bright future for you on StackOverflow and the yet unrevealed Documentation ;) keep up the good work soldier ;) – Shark Apr 18 '16 at 14:42