0

Here is my program to uppercases all letter inputted from the standard input. But some output is very strange. For example, if input is "lorem ipsum" and output will be "LOREM IPSUMS?". If input is a single character such as 'm', the output will be "MZ#X?" . "S?" and "Z#X?" should not be here but they append to the output.

Why this happens?

#include <stdio.h>
#include <ctype.h>

int main(void){
        char input;
        char upper[100];
        int count = 0;

        while((input = getchar())){
                if(input == '\n')
                        break;
                if(input >= 'a' && input <= 'z')
                        input = toupper(input);
                *(upper + count) = input;
                count++;
        }

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

        return 0;
}
Jennifer Q
  • 257
  • 3
  • 12
  • 5
    You didn't null-terminate your string. – user2357112 Mar 17 '16 at 22:06
  • A string in C needs an end marker to print properly. – tofro Mar 17 '16 at 22:07
  • 1
    Also worth noting [`getchar()`](http://en.cppreference.com/w/c/io/getchar) returns EOF on, well, end of file (including terminal input if done there). Consequently, without a newline (such as a redirect from a file with a single non-newline-terminated string or someone pressing ctrlz/d on the terminal keyboard) this will loop forever. Not sure how strict the adjudicator of this is going to be, but you may want to fix that. – WhozCraig Mar 17 '16 at 22:19

3 Answers3

2

Your code is working for me. The only thing, but this is expected, you get garbage after the string, because your char array has a length of 100. You should put a 0 at the end of the string to tell printf your string ends there. put

*(upper + count) = 0;

right before printf.

Mario Dekena
  • 843
  • 6
  • 20
1

Your code has a some issues:

  • the NULL terminator isn't added at the end of the string.
  • There isn't any check of index out of bounds of the allocated array.
  • User may interrupt input without a newline making the program loop forever.
  • getchar returns an int.

You can try this fix:

#include <stdio.h>
#include <ctype.h>

const int MAX_CHAR = 101;

int main() {
    int input;
    char upper[MAX_CHAR];
    int count = 0;

    while( count < MAX_CHAR - 1 ) {
        input = getchar();
        if ( input == EOF  || input == '\n' ||  input == '\r' )
            break;
        if ( input >= 'a' && input <= 'z' )
            input = toupper(input);
        upper[count] = input;
        count++;
    }
    // add the null terminator
    upper[count] = '\0';

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

    return 0;
}
Bob__
  • 12,361
  • 3
  • 28
  • 42
0

You might want to set all the elements in your array to be 0 (ASCII NUL) because we have no idea what is in the upper array.

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

int main(void){
    char input;
    char upper[100];
    int count = 0;

    memset(upper, 0, 100);

    while((input = getchar())){
            if(input == '\n')
                    break;
            if(input >= 'a' && input <= 'z')
                    input = toupper(input);
            *(upper + count) = input;
            count++;
    }

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

    return 0;
}

If you are not sure what memset does, you can do it with a for loop.

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

int main(void){
    char input;
    char upper[100];
    int count = 0;

    for (int i = 0; i < 100; i++) {
       upper[i] = 0;
    }

    while((input = getchar())){
            if(input == '\n')
                    break;
            if(input >= 'a' && input <= 'z')
                    input = toupper(input);
            *(upper + count) = input;
            count++;
    }

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

    return 0;
}
Warden
  • 106
  • 5
  • Is **count** actually used or needed ? – Arif Burhan Mar 17 '16 at 22:13
  • 2
    Is there some reason *not* to use `upper[count] = 0;` after the while-loop rather than blast over a memory region with nullchar when only *one* is required, *and you already know where it belongs*? – WhozCraig Mar 17 '16 at 22:14
  • OP uses count to index back to the `upper` array. – Warden Mar 17 '16 at 22:14
  • Sell that 99 '0' bytes for good money. One of them works well enough ;) – tofro Mar 17 '16 at 22:15
  • @WhozCraig, my thoughts exactly. +1 to that comment. There's no sense making **EVERYTHING** null. That's just unnecessary processing operations on the CPU. – Spencer D Mar 17 '16 at 22:16