Thread: Please review my text concatenation try in C

Hi

I am trying to understand the strings in C so I tried to implement 3 different versions of text concatenation in C. I would love to hear



-what would be the best approach(array vs pointers based on my try)

-Any way to improve any of the approaches I tried?

-In the "void *concText_p" function how can I "free" the newText pointer before returning it? Or what would be the best way to use malloc and concatenation in the same function as the way I am trying to do?


1) uses array

2) I try using pointers and returning a pointer

3) I try using pointers and modify the original incoming variable.

Code:

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

#define STRLEN 512
char *concText(char *source1,  char *source2);
char *concText_p(char *source1,  char *source2, size_t len);
void *concText_p2(char *target, char *source1,  char *source2);

void main() {
    char *source1="source1-0123456789";
    char *source2="source2-9876543210";
    char *result=(char *)malloc(512*sizeof(char)); //TODO remove the constant charlength here
        printf("(source1)-> %s  (source2)-> %s  len -> %d\n\n",source1,source2, (strlen(source1)+strlen((source2))));

    result=concText(source1,source2);
        printf("[MAIN] => Merged Text is %s  len is %d \n",result,strlen(result));

//    result=concText_p(source1,source2,STRLEN);
//        printf("[MAIN] => Merged Text is %s  len is %d",result,strlen(result));

//    concText_p2(result, source1,source2);
//        printf("[MAIN] => Merged Text is %s  len is %d \n",result,strlen(result));    z

}

char *concText(char *source1,  char *source2){
    int count=0;
    char *tmpSource1=source1;
    char *tmpSource2=source2;

    char newText[STRLEN];

    while(*(tmpSource1)!='\0'){
        newText[count]=*tmpSource1++;
        count++;
    }

    while(*(tmpSource2)!='\0'){
        newText[count]=*tmpSource2++;
        count++;
    }
        newText[count]='\0';
    return newText;
}


char *concText_p(char *source1,  char *source2, size_t len){
    int count=0;
    char *tmpSource1=source1;
    char *tmpSource2=source2;

    char *newText=malloc(len);
        if (newText==NULL){printf("It cannot allocate enough memory"); return EXIT_FAILURE;}
    while(*(tmpSource1)!='\0'){
        *newText++=*tmpSource1++;
        count++;
    }
    while(*(tmpSource2)!='\0'){
        *newText++=*tmpSource2++;
        count++;
    }
        *newText='\0';
    //Rollback to the starting adress of the text array
    while(count-->0){ newText--;}

    return newText;
}

void *concText_p2(char *target, char *source1,  char *source2){
    int count=0;
    char *tmpSource1=source1;
    char *tmpSource2=source2;

    char *newText=target;

    while(*(tmpSource1)!='\0'){
        *newText++=*tmpSource1++;
        count++;
    }

    while(*(tmpSource2)!='\0'){
        *newText++=*tmpSource2++;
        count++;
    }
        *newText='\0';
}

Don't bother with the tmpSource1 and tmpSource2 variables. Just use the source1 and source2 parameters.

You shouldn't be returning the address of local variable newText in concText. So even though it may SEEM to work, this method is no good.

Don't "roll back" the newText var in concText_p. Instead, create a new variable, char *p = newText; after the malloc test and use p to walk through the array and pass back newText.

This makes no sense:
char *newText=malloc(STRLEN*len);
You don't even need to pass len into that function. So get rid of it and just do this:
char *newText=malloc(strlen(source1)+strlen(source2)+1) ;

Almost forgot, you return EXIT_FAILURE from a function that's supposed to return a char*. You should just pass back NULL.