Print with junk values,c

Question

I'm having a problem and I can not figure out why. In main I create 2 arrays of strings. The function accepts 2 strings and creates a new array of the appropriate size according to the following requirement: For example: Array 1: aviv , Array 2: 12 , The new array: a12v12i12v The new array must be exactly the size! And then send the new array to main and main to print it. I also print junk values. I checked the size of the new array and it is the right size. My code:

 char* CreateString(char* str1, char* str2)
    {
        int length1, length2, length3 = 0;
        int i,j, index_help = 0;
        char *str3 = NULL;
        length1 = strlen(str1);
        length2 = strlen(str2);
        for (i = 0; i < length1; i++) // Check the size of the new array

        {
            length3++;
            if (i == (length1 - 1))
            {
                break;
            }
            for (j = 0; j < length2; j++)
            {
                length3++;
            }
        }
        str3 = (char*)malloc(length3+1  * sizeof(char));
        if (str3 == NULL)
        {
            printf("There is not enough memory space\n");
            return 0;
        }
        for (i = 0; i < length1; i++) //Copying data

        {

            str3[index_help] = str1[i];
            if (i == (length1 - 1))
            {
                break;
            }
            for (j = 0; j < length2; j++) 
            {
                index_help++;
                str3[index_help] = str2[j];
            }
            index_help++;

        }
        return str3;
    }

    int main()
    {
        char *str1 = NULL, *str2 = NULL,*str4=NULL;
        int size1, size2,i;
        printf("enter the size of string number 1:\n");
        scanf("%d", &size1);
        printf("enter the size of string number2 :\n");
        scanf("%d", &size2);
        str1 = (char*)malloc((size1 + 1) * sizeof(char));
        if (str1 == NULL)
        {
            printf("There is not enough memory space\n");
            return 0;
        }
        str2 = (char*)malloc((size2 + 1) * sizeof(char));
        if (str2 == NULL)
        {
            printf("There is not enough memory space\n");
            return 0;
        }
        printf("Enter a value for the first string (the size is:%d):\n", size1);
        scanf("%s", str1);
        printf("Enter a value for the second string (the size is:%d):\n", size2);
        scanf("%s", str2);
        str4 = CreateString(str1, str2);
printf("%s",str4);
        printf("\n");
        return 0;
    }

`for (i = 0; i < length1; i++)` - The first `for` loop - What is the logic here? You do not need a loop to achieve what it is doing — Ed Heal, Dec 25 '17 at 16:56
junk chars at end of string: dead giveaway for forgetting to null terminate string. — Jean-François Fabre, Dec 25 '17 at 16:56
`str3[index_help] = '\0';` just before returning it. That'll do — Jean-François Fabre, Dec 25 '17 at 16:57
@EdHeal the first loop is to count how mach chars i need to the new arry (size) — aviv.L, Dec 25 '17 at 17:04
Casting malloc is bad - See https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc — Ed Heal, Dec 25 '17 at 17:04
Perhaps try calculating the size of te resulting array with some kind of mathematical formula. Repeated addition... hmm... sounds vaguely familiar. — n. 1.8e9-where's-my-share m., Dec 25 '17 at 17:06
@EdHeal You're right, it can be solved mathematically. But I wanted to solve the exercise with loops. — aviv.L, Dec 25 '17 at 17:08
Avoid loops. Anyway what is with this nonsense `if (i == (length1 - 1))` ? That could be put in the for condition — Ed Heal, Dec 25 '17 at 17:10
Your malloc call is just wrong. 1. Do not cast it. 2. Use the proper arithmetic: `length3+1 * sizeof(char)` is not the same as `(length3+1) * sizeof(char)`. In this case you are lucky, as `sizeof char` is 1. But let's assume you are dealing with `int`s, then it just yields the wrong number of bytes. — Pablo, Dec 25 '17 at 17:14
@Ed Heal Casting malloc is bad only for low-qualified programmers. Please do not provide references to very bad answers. — Vlad from Moscow, Dec 25 '17 at 17:44
@Pablo You are mistaken. (length3+1) * sizeof(char) is equivalent to length3+1 * sizeof(char) and in turn is equivalent to length3+1 — Vlad from Moscow, Dec 25 '17 at 17:45
@Vlad in the case of `sizeof(char)` that's true, because `sizeof(char)` is 1. I said that. I also said, let's assume it were int. Then `(length3+1) * sizeof(int)` and `length3+1 * sizeof(int)` are **not** the same. That was my point — Pablo, Dec 25 '17 at 18:07

score 0 · Answer 1 · answered Dec 25 '17 at 17:54

The function should be declared like

 char* CreateString( const char* str1, const char* str2);

because neither string str1 nor string str2 are changed in the function.

You have to append the result string with a terminating zero.

The length of the resulted string can be calculated simpler without using loops.

As the function strlen has the return type size_t then variables that accept the result of a function call should be also declared as having the type size_t instead of the type int.

The function can be implemented the following way as it is shown in the demonstrative program.

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

char * CreateString(const char *s1, const char *s2)
{
    size_t n1 = strlen(s1);
    size_t n2 = strlen(s2);

    size_t n3 = n1 == 0 ? 0 : n1 + (n1 - 1) * n2;

    char *s3 = ( char * )malloc(n3 + 1);

    if (s3)
    {
        if (n2 == 0)
        {
            strcpy(s3, s1);
        }
        else
        {
            char *p = s3;

            while (*s1)
            {
                *p++ = *s1++;
                if (*s1)
                {
                    strcpy(p, s2);
                    p += n2;
                }
            }

            *p = '\0';
        }
    }

    return s3;
}

int main( void )
{
    char *p = CreateString("aviv", "12");

    printf("\"%s\"\n", p);

    free(p);

    p = CreateString("", "12");

    printf("\"%s\"\n", p);

    free(p);

    p = CreateString("a", "12");

    printf("\"%s\"\n", p);

    free(p);

    p = CreateString("av", "12");

    printf("\"%s\"\n", p);

    free(p);

    p = CreateString("av", "");

    printf("\"%s\"\n", p);

    free(p);

    return 0;
}

The program output is

"a12v12i12v"
""
"a"
"a12v"
"av"

Print with junk values,c

1 Answers1