C program outputs weird characters

Question

Trying to print reversed input in C:

#include <stdio.h>

#define MAX_SIZE 100
/* Reverses input */
void reverse(char string[], char reversed[]);

int main() {
    char input[MAX_SIZE], output[MAX_SIZE + 1];
    char c;
    int index = 0;

    while ((c = getchar()) != '\n')
    {
        input[index] = c;
        ++index;
    }

    reverse(input, output);
    printf("%s\n", output);

}

void reverse(char string[], char reversed[]) {  
    int rev;
    rev = 0;

    for (int str = MAX_SIZE - 1; str >= 0; --str) {
        if (string[str] != '\0') {
            reversed[rev] = string[str];
            ++rev;
        }
    }
}

but have this weird result:

input:

abc

output:

?:? ????:???:?cba?

both input and output arrays comprise \0, so I guess there's some index out of bounds exception, but I can't find out where exactly. Thank you.

You need to pass the `index` to `reverse` so that you can use the actual size and not MAX_SIZE. — 500 - Internal Server Error, Apr 19 '21 at 10:40
You know the size of the string before you try to reverse it. Why not use it in the reverse function (`void reverse(char string[], char reversed[], size_t len)`)? You're reversing `MAX_SIZE` chars where only `index` chars need to be reversed. — pmg, Apr 19 '21 at 10:42
Also, the code shown has no guarantee for any of the arrays to contain a `'\0'` — pmg, Apr 19 '21 at 10:47
@500-InternalServerError thank you, works now, but I still can't understand why I get these random characters in output. — Oleksandr Novik, Apr 19 '21 at 10:52
@pmg thanks as well, and yeah, you're right about `\0`. I forgot about garbage values — Oleksandr Novik, Apr 19 '21 at 10:53

anastaciu · Accepted Answer · 2021-04-19T12:16:27.407

1

For the length of the original string you shouldn't use MAX_SIZE because that's the total size of the container, not the size of the string.

Another problem is that the input string is not null terminated, and because of that it's not possible to know its length, unless you tracked the number of charaters read from stdin and passed it as an argument.

Fixing these two main issues (along with some other minor problems (comments)) would make your code work:

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

#define MAX_SIZE 100

void reverse(const char string[], char reversed[]);

int main()
{
    char input[MAX_SIZE], output[MAX_SIZE]; // no need for the extra character
    char c;
    int index = 0;

    while ((c = getchar()) != '\n' && index < MAX_SIZE - 1) // avoid buffer overflow
    {
        input[index] = c;
        ++index;
    }
    input[index] = '\0'; // null terminate the original string

    reverse(input, output);
    printf("%s\n", output);
}

void reverse(const char string[], char reversed[])
{
    int rev;
    rev = 0;

    // stop condition with the length of the string
    for (int str = strlen(string) - 1; str >= 0; --str)
    {
        reversed[rev] = string[str];
        ++rev;
    }
    reversed[rev] = '\0'; // null terminate the reversed string
}

edited Apr 19 '21 at 12:16

answered Apr 19 '21 at 11:03

anastaciu

23,467
7
28
53

are reversed[] and string[] like *string and *reversed? How is value passed back to main? – EMS Apr 19 '21 at 11:07
@EMS, yes, [arguments like `char string[]` decay to `char *string`](https://stackoverflow.com/q/1461432/6865932), so being pointers, the changes are permanent and accessible outside the function. – anastaciu Apr 19 '21 at 11:09
I was taught in string[] 'string' is a constant pointer to a array of chars. Wouldn't that mean string is a constant address? And passing another address to it should not be possible? – EMS Apr 19 '21 at 11:14
@EMS if the array is initialized with `[]` ***i.e.*** `char string[] = "some string"` the array will contain a copy of the string literal and thus it can be changed, however if it's initialized as a pointer ***i.e.*** `char *string = "some string"` , then it's a pointer to a string literal and trying to change it is undefined behavior. – anastaciu Apr 19 '21 at 11:19
@EMS I find [this answer to another post](https://stackoverflow.com/a/30662565/6865932) very clear on explaining how these work. – anastaciu Apr 19 '21 at 11:23
I have read this one before. My question is more about how changes to reverse[] are passed back to output[] as reverse may decay to *reverse but in reality it is a constant pointer? As per my understanding data of input and output is copied to string and reverse instead of copying addresses of input and output to string and reverse. – EMS Apr 19 '21 at 11:30
Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/231314/discussion-between-ems-and-anastaciu). – EMS Apr 19 '21 at 11:36
@EMS the data of `output` and `input` is not copied, when you use `reverse(input, output);`, you are passing two pointers one to `input` and another to `output`, the changes made to them are permanent precisely because of that. In fact the function signature could be `void reverse(char *string, char *reversed)`, and better, the first argument could be `const` as not changes are made to the oblect it points to. – anastaciu Apr 19 '21 at 11:38
1

That is what I wanted to know. thank you:-) – EMS Apr 19 '21 at 11:42
@EMS, glad to help. – anastaciu Apr 19 '21 at 11:43

Vlad from Moscow · Answer 2 · 2021-04-19T11:21:19.203

For starters it is unclear why the array input has MAX_SZIE elements

char input[MAX_SIZE], output[MAX_SIZE + 1];

while the array output has MAX_SIZE + 1 elements.

After this loop (that is unsafe)

while ((c = getchar()) != '\n')
{
    input[index] = c;
    ++index;
}

the array input does not contain a string.

The initial value of the variable str in this loop within the function revrese

for (int str = MAX_SIZE - 1; str >= 0; --str) {

does not make a sense because the user can enter less than MAX_SIZE - 1 characters in the array.

The program in whole and the function reverse can look the following way as it is shown in the demonstrative program below.

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

#define MAX_SIZE 100

char * reverse( const char src[], char dsn[] )
{
    char *p = dsn + strlen( src );

    *p = '\0';

    while (p != dsn) *--p = *src++;

    return dsn;
}

int main( void )
{
    char input[MAX_SIZE], output[MAX_SIZE];

    size_t i = 0;

    for (int c; i + 1 < MAX_SIZE && ( c = getchar() ) != EOF && c != '\n'; ++i)
    {
        input[i] = c;
    }

    input[i] = '\0';

    puts( reverse( input, output ) );
}

If to enter for example text

Hello World!

then the output will be

!dlroW olleH

Pay attention to that the first function parameter should have the qualifier const because the passed source string is not being changed within the function. The function should follow the common convention for standard string functions and return a pointer to the destination string. That is the function return type void does not make a great sense.

And when you deal with strings use objects of the unsigned type size_t instead of the signed type int as indices. The type size_t is the type of returned values of the function strlen or the operator sizeof.

C program outputs weird characters

2 Answers2