Garbage characters are printed forcefully

Question

This is a program that's supposed to read inputs, a number 'n' and a character, and then duplicate this character n times. It works perfectly fine, but when I enter a large number, 8+ for example, it duplicates perfectly but then adds garbage values to the end. I can't get why it does that since I used malloc and I have exactly n blocks saved for me in the memory.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
char* create_string (char ch, int n);
void main ()
{
    int n;
    char ch;
    printf("Enter number for duplicates: ");
    scanf("%d",&n);
    printf("Enter a letter: ");
    scanf(" %c", &ch);
    printf("The letter '%c' duplicated %d times is: ",ch,n);
    char* ptr=create_string(ch,n);
    printf("%s",ptr);
}
char* create_string (char ch, int n)
{
    char* dup=(char*)malloc(n*sizeof(char));
    int i;
    for (i=0; i<n; i++)
    {
        dup[i]=ch;
    }
    return dup;
}

Test run: Test run.

Your `create_string` function does not create a string. – melpomene Dec 08 '18 at 09:24 — melpomene, Dec 08 '18 at 09:24
I cannot see the "Test run" pic. – alk Dec 08 '18 at 09:44 — alk, Dec 08 '18 at 09:44

iBug · Accepted Answer · 2018-12-08T09:30:52.227

3

Strings in C are as simple as null-terminated character sequences. That means whenever you create a string by hand, you must always append a '\0' at the end so other functions like printf know where it ends:

char* create_string (char ch, int n)
{
    char* dup = malloc((n+1) * sizeof(char));
    int i;
    for (i=0; i<n; i++)
    {
        dup[i]=ch;
    }

    // This is important
    dup[n] = '\0';

    return dup;
}

Another subtle thing to notice is that because you need to store that terminating null character, you also need to reserve the space for it. So the malloc line is changed into:

malloc((n+1)*sizeof(char))
//     ^^^^^ it's no longer 'n'

On a side note, you don't need to cast the returned pointer of malloc.

edited Dec 08 '18 at 09:30

answered Dec 08 '18 at 09:27

iBug

35,554
7
89
134

@KeineLust Looks like I missed that point. Thanks – iBug Dec 08 '18 at 09:28
2

Multiplying by `sizeof (char)` is pointless. It's 1 by definition. – melpomene Dec 08 '18 at 09:30
@melpomene Keeping that is harmless and may sometimes be beneficial in terms of readability for beginners. – iBug Dec 08 '18 at 09:31
1

Personally, for beginners, I think it is good to show you are setting the adequate size based on (1) the number of elements needed; and (2) the type-size of the object or variable. From that standpoint it probably does more good than harm. – David C. Rankin Dec 08 '18 at 09:33
Remember, we can always use sizeof( *dup ); . There a missing free() call also. – Michi Dec 08 '18 at 09:43
Yes, where do I add the free() call? – Youssef Hussein Dec 08 '18 at 11:23
@YoussefHusseinY When you don't need it anymore. In your case, probably after the `printf` call. – iBug Dec 08 '18 at 12:57

score 1 · Answer 2 · answered Dec 08 '18 at 09:27

Strings in C are char arrays where the character \0 denotes the end of the string. Since you aren't explicitly adding it, printf just prints values from memory until it happens to run in to a terminating character (i.e., this is undefined-behavior). Instead, you should explicitly add this character to your result string:

char* create_string (char ch, int n)
{
    char* dup = (char*) malloc((n + 1) * sizeof(char));
    /* Added 1 for the '\0' char ---^ */

    int i;
    for (i = 0; i < n; i++)
    {
        dup[i]=ch;
    }

    /* Set the terminating char */
    dup[n] = '\0';

    return dup;
}

Garbage characters are printed forcefully

2 Answers2