How to get rid of garbage characters in this program?

Question

This program is supposed to print an input string backwards. Every single time it happens, though, I get garbage characters such as \340 or of the like. Why is it doing that? Here's my code:

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

int main()
{
    char mattsentence[51];
    mattsentence[50] = '\0';
    gets(mattsentence);
    char mask[sizeof(mattsentence)];
    int i, j;
    j = sizeof(mattsentence);
    for (i = 0; i < sizeof(mask); i++)
    {
        j = j - 1;
        mask[i] = mattsentence[j];
        printf("%c", mask[i]);
    }
    printf("\n");
}

You enjoyed garbage characters because you failed to initialize your variables. `char mattsentence[51] = {0};` The same applies to the remainder of your variables as well. If they are not global and you don't initialize them, then there is no telling what you will find inside. Additionally, any attempt to read from an uninitialized variable is **undefined behavior**. — David C. Rankin, Mar 30 '15 at 06:42
Don't use `gets()`: [Why is the `gets()` function dangerous?](http://stackoverflow.com/questions/1694036/why-is-the-gets-function-dangerous-why-should-it-not-be-used) — Jonathan Leffler, Mar 31 '15 at 02:17

Sourav Ghosh · Answer 1 · 2015-03-30T08:30:23.257

sizeof() operator gives the size of the datatype. So, sizeof(mattsentence) will give you a value of 51. Then, sizeof(mask) will give you 51 again.

When you use that sizeof(mask) as for loop condition, you're basically going past the actual input values, thus pritning out garbage values.

What you want here is to use strlen() to find out the actual valid length of the entered string.

So, basically you need to take care of

Point 1: replace sizeof with strlen().

Point 2: Use of gets() is dangerous. Please use fgets() instead of gets().

Point 3: int main() should be int main(void). Put an expilicit return statement at the end of main(). Good Practice.

The modified code should look like

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

int main(void)
{
    char mattsentence[51] = {0};                  //always initalize local variables, here it's covering null termination , too.

    fgets(mattsentence, 51, stdin);              //fgets()

    char mask[strlen(mattsentence) + 1];         // one more to store terminating '\0'

    int i = 0, j = 0, k = 0;
    j = strlen(mattsentence);
    k = j;
    for (i = 0; i < k; i++)  // make use of k, don't call `strlen()` repeatedly
    {
        j = j - 1;
        mask[i] = mattsentence[j];
        printf("%c", mask[i]);
    }
    mask[i] = '\0'; // for proper string termination
    printf("\n");
    printf("%s\n", mask);

    return 0;                     //added return statement
}

Null termination becomes redundant since you initialized the array to zero :) — Quentin, Mar 30 '15 at 07:54

Vlad from Moscow · Accepted Answer · 2015-03-31T05:19:54.947

2

Your approach is wrong because you reverse the entire character array while it can be filled only partially. You should use standard C function strlen declared in header <string.h> that to determine the size of the entered string. Also to use gets is unsafe because you can overwrite memory beyond the character array. It now is excluded from the C Standard

Here is shown how the program can be written.

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

#define N   51

int main(void) 
{
    char mattsentence[N] = { '\0' };
    char mask[N] = { '\0' };

    fgets( mattsentence, sizeof( mattsentence ), stdin );

    size_t n = strlen( mattsentence );

    if ( n != 0 && mattsentence[n-1] == '\n' ) mattsentence[--n] = '\0';

    for ( size_t i = 0; n != 0; i++ )
    {
        mask[i] = mattsentence[--n];        
        printf( "%c", mask[i] );
    }

    printf( "\n" );

    return 0;
}

If to enter

Hello, Christiana S. F. Chamon

then the program output will be

nomahC .F .S anaitsirhC ,olleH

Take into account that to output a string in the reverse order there is no need to define a second character array.

If you want only to output the source string in the reverse order then the program can look like

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

#define N   51

int main(void) 
{
    char mattsentence[N] = { '\0' };

    fgets( mattsentence, sizeof( mattsentence ), stdin );

    size_t n = strlen( mattsentence );

    if ( n != 0 && mattsentence[n-1] == '\n' ) mattsentence[n-1] = '\0';

    while ( n != 0 )
    {
        printf( "%c", mattsentence[--n] );
    }

    printf( "\n" );

    return 0;
}

edited Mar 31 '15 at 05:19

answered Mar 30 '15 at 05:39

Vlad from Moscow

301,070
26
186
335

I've always been lazy and preferred just `char mattsentence[N] = { 0 };` It just save a couple of characters. I know you know, but it is worth pointing out that they are equivalent. – David C. Rankin Mar 30 '15 at 06:38
@ David C. Rankin Are you indeed an Engineer and lawyer? – Vlad from Moscow Mar 30 '15 at 08:02
Yes, B.S. Aerospace Engineering 1990 Texas A&M, P.E. Registration in Texas 1995, J.D. South Texas College of Law 1996. (blending engineering an law is enough to make anyone bipolar `:p`) – David C. Rankin Mar 30 '15 at 08:04
@David C. Rankin As I understand you are now more a lawyer than engineer that is your main task is to deal with the law. – Vlad from Moscow Mar 30 '15 at 08:15
Yes, I am a lawyer full-time. Left NASA in 1995 and have been in law since. Code is my chess. – David C. Rankin Mar 30 '15 at 08:16
@David C. Rankin Do you play also chess? – Vlad from Moscow Mar 30 '15 at 08:20
Yes, but I'm not an accomplished player. Shoot me an e mail. david at rankinlawfirm.com and we'll follow up tomorrow. – David C. Rankin Mar 30 '15 at 08:29
Why not `char mattsentence[N] = "";`? It is shorter than either! Also, in the code, don't you need to decrement `n` when you overwrite the newline? – Jonathan Leffler Mar 31 '15 at 02:05
@Jonathan Leffler Thanks. About decreasing n then it was a typo.:) As for array initialization then I prefered to use the general form of initialization that can be applied to an array of any type.. – Vlad from Moscow Mar 31 '15 at 05:23

Anto Jurković · Answer 3 · 2015-03-30T05:35:35.267

See changed code:

int main()
{
    char mattsentence[51];
    mattsentence[0] = '\0'; // initialization

    gets(mattsentence);

    char mask[strlen(mattsentence) + 1]; // +1 for string terminator '\0'

    int i, j;
    j = strlen(mattsentence);
    for (i = 0; i < strlen(mattsentence); i++)  // strlen of original string
    {
        j = j - 1;
        mask[i] = mattsentence[j];
        printf("%c", mask[i]);
    }
    mask[i] = '\0'; // for proper string termination
    printf("\n");
    printf("%s\n", mask);
}

There are several errors:

strlen() should be used to get length of string
for loop should be controlled according to input string, not output string
it is better to use fgets() instead of gets(): that way you can control how many character will be read from the input

The initialization of `mattsentence` is unnecessary; `gets()` — which should be replaced by `fgets()` — null terminates the string it reads and overwrites the buffer it is pointed at. You should, in theory, test the return value from `gets()` to detect EOF; I suppose the initialization means that you get a zero-length string, so you can get away with it (and that makes the initialization necessary), but it feels a little clunky. — Jonathan Leffler, Mar 31 '15 at 02:20

How to get rid of garbage characters in this program?

3 Answers3