Problem with passing back pointer pointed to string

I am trying to learn C programming, and I am doing an exercise that

I need write a function that accepts two strings. Use the malloc() function to allocate enough memory to hold the two strings after they have been concatenated (linked). Return a pointer to this new string.

For example, if I pass "Hello " and "World!", the function returns a pointer to "Hello World!".

and I wrote the following code,

Code:

---

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

#define LEN 20
//const short int LEN = 20;
char *stringConcat(char s1[], char s2[]);

int main(void)
{
       
        char str1[LEN], str2[LEN];
        char *ptrString;

        printf("Enter a value string 1:");
        scanf("%20s", str1);

        printf("Enter a value string 2:");
        scanf("%20s", str2);
       
        ptrString = stringConcat(str1, str2);

        printf("The two string are now concated, it becomes\n%s\n\n",ptrString);
        return 0;
}
char *stringConcat(char s1[], char s2[])
{
        int i1 = 0, i2=0,i=0;
        //local pointer;
        char *ptr1=NULL;
       
        while(s1[i1]!='\0')
                i1++;

        while(s2[i2]!='\0')
                i2++;
       
        ptr1 = (char *) malloc(i1+i2+1 * sizeof(char));



        for (i=0; i<i1; i++)
        {
                ptr1++ = s1[i]; //the problem must be here
        }

       
        for (i=0; i<i2; i++)
        {
                ptr1++ = s2[i]; ////the problem must be here
        }
       

        printf("%s", ptr1); // you would see sth unreadable
        return ptr1; // the returned values would be unreadable Ascii codes
     


}

---

Since you're incrementing ptr1, at the end of the function it's pointing past the end of the string. You should declare a second pointer for the purpose of incrementing. Another problem due to this is there is no way to free the original memory that was malloc'ed.

Also the return value of malloc() shouldn't be casted.

> ptr1++ = s1[i]; //the problem must be here
Also with this line you never actually copy the char, all you do is increment the pointer. I'm actually surprised it compiles.

Quote:

---

Originally Posted by swoopy

> ptr1++ = s1[i]; //the problem must be here
Also with this line you never actually copy the char, all you do is increment the pointer. I'm
actually surprised it compiles.

---

if I wrote

Code:

---

*ptr1++ = s1[i];

---

or

Code:

---

ptr1++ = &s1[i];

---

then my VS 2005 wont compile at all. It makes me so wondered in that I cannot assign the memory of the array element

Code:

---

&s[i]

---

to the pointer

Code:

---

ptr1

---

HOWEVER, I can write like this

Code:

---

*ptr1++='c'

---

then it's ok BUT I meet such problem

>*ptr1++ = s1[i];
Are you sure that won't compile? What error message does the compiler emit?

Just because *ptr++ = 'c' compiles doesn't mean it is a good idea in this case. ptr1 points to allocated memory, and if you move the pointer from the start of that block, then you cannot necessarily free it properly.

ptr1++ is not part of an expression, it is an expression on it's own. In pointer arithmetic, we moved to the next address: ptr1 = ptr1 + 1.

If you just use ptr[index] and increment the index then the problem goes away. You don't want to move what the pointer points to anyway because you need to return the front of the string. So the code becomes:

Code:

---

char * stringCopy( const char s1[], const char s2[] )
{
  char * retval = NULL;
  size_t i = 0;
  size_t s1len = 0;
  size_t s2len = 0;

  while( s1[s1len] != '\0' ) ++s1len;
  while( s2[s2len] != '\0' ) ++s2len;

  retval = malloc( 1 + s1len + s2len );
  if( retval != NULL ) {
      for( i = 0; i < s1len; ++i ) retval[i] = s1[i];
      for( i = 0; i < s2len; ++i ) retval[s1len + i] = s2[i];
      retval[s1len + s2len] = '\0';
  }
  return retval;
}

---

Quote:

---

Originally Posted by citizen

Just because *ptr++ = 'c' compiles doesn't mean it is a good idea in this case. ptr1 points to allocated memory, and if you move the pointer from the start of that block, then you cannot necessarily free it properly.

ptr1++ is not part of an expression, it is an expression on it's own. In pointer arithmetic, we moved to the next address: ptr1 = ptr1 + 1.

---

That's exactly why it's a bad idea to try to do too many things in a single statement. It's too easy to miss some little detail...

Quote:

---

Originally Posted by whichet

...
I wrote the following code

---

So you did. Good for you!.

Oh, did you have an actual question? You know, like with a question mark and all, like this?

another problem,
ptr1 = (char *) malloc(i1+i2+1 * sizeof(char));
//here,u should allocate (i1 + i2) byte memry for these two strings,because there is no "\0" added to the end of a char Array;diffrent from the string.

so if your program run ok,the last char it print will be something uncertain