C Dynamic String Allocation

[souradipm]

Hi,
I am using C to make something, and part of it requires dynamic string allocation. It works, but not perfectly. The demo below prints "Hell World" instead of "Hello World". Please help me to find anything wrong in it.

/*
Dynamic String Allocation
By Five Point Software 2008
*/

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

char *ConcatStr(char *old_str, char *append_str)
{
  int num_chars=strlen(old_str)+strlen(append_str);
  char *new_str=malloc(num_chars);
  int i,s;
  for(i=0;i<num_chars;i++){
      *(new_str+i)=0;
  }
  for(i=0;i<strlen(old_str)-1;i++){
      *(new_str+i)=*(old_str+i);
  }
  s=i;
  for(i=0;i<strlen(append_str);i++){
      *(new_str+i+s)=*(append_str+i);
  }
  free(old_str);
  return new_str;
}

char *NewStr(char *initial_str)
{
  int num_chars=strlen(initial_str);
  char *new_str=malloc(num_chars+1);
  int i;
  for(i=0;i<num_chars;i++){
      *(new_str+i)=0;
  }
  strcpy(new_str,initial_str);
  return new_str;
}

void FreeStr(char *old_str)
{
     free(old_str);
}

int main(){
    char *s1=NewStr("Hello");
    char *s2=ConcatStr(s1," world");
    printf(s2);
    FreeStr(s2);
    getch();
}

Btw, I found the skeleton of the code on the page of a C programming course, with just /* FILL IN THIS FUNCTION */ instead of any real code. I was bored so I filled it in. I don't see what is wrong with it, so if you could spot anything wrong, please tell me.

~souradipm

 

[quantum1]

I believe the code
for(i=0;i<strlen(old_str)-1;i++){
should be
for(i=0;i<strlen(old_str);i++){

 

[natsuki]

It's what quantum said. But if you simply remove the -1 in it you might end up crashing. It's like a bug. When you dynamically allocate strings ALWAYS make sure it's the length of the string you want + 1 or else your string will continue till it eats up all your memory or it crashes. It's because you need to null terminate (with null character '\0' or 0) so it knows the string has ended.

char *ConcatStr(char *old_str, char *append_str)
{
  [COLOR="Red"]int num_chars=strlen(old_str)+strlen(append_str);
  char *new_str=malloc(num_chars); // should be + 1[/COLOR]
  int i,s;
  for(i=0;i<num_chars;i++){
      *(new_str+i)=0;
  }
  for(i=0;i<strlen(old_str)[COLOR="Red"]-1[/COLOR];i++){ // this one luckily saves you from your mistake though you didn't expect a mistake will save you from even a bigger mistake above
      *(new_str+i)=*(old_str+i);
  }
  s=i;
  for(i=0;i<strlen(append_str);i++){
      *(new_str+i+s)=*(append_str+i);
  }
  free(old_str);
  return new_str;
}

char *NewStr(char *initial_str)
{
  int num_chars=strlen(initial_str);
  char *new_str=malloc(num_chars+1);

I wonder why in this you added + 1 but in the other one you didn't.. lol
And you could just use new_str instead, the dereferencing * thing is kinda bulky.

In php it's ok to make a mistake but in c it's doom. lol

Maybe you want to try out my programming language just for fun lol.