1

I am practicing how to code C programs, specifically reading stdin statements. The following is a code I wrote to take in the stdin, but I am having trouble inputting them into an array and printing out the correct values.

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

int main(int argc, char* argv[])
{
    int count = 0;
    char* number = "-n";
    int result;
    char numArray[50];

    result = strcmp(argv[1], number);
    if (result == 0) {
        printf("Numbers Only\n");
        while (!feof(stdin)) {
            if (feof(stdin))
                break;

            for (int i=0; i < sizeof(numArray); i++){
                scanf("%s", &numArray[i]);
            }
        }
    }
    for (int i=0; i< sizeof(numArray); i++){
        printf("%d\n", numArray[i]);
    }
}

I am working step by step, so my final code has something to do with manipulating the array and outputting it. However, my question is focusing solely on inputting the stdin into the array first since that is the big step and I will work on manipulating the array later.

The 'Numbers Only' is what I was using to check something out, so do not worry about that at all.

I do not get any errors for the code, but it gives weird outputs. One output is the following:

1                  (1, 2, 3 are what I inputted into terminal)
2
3
49
50
51
0
0
0
0
0
32
-56
109
-63
6
127
0
0
-128
80
110
-63
6
127
0
0
20
0
0
0
0
0
0
0
64
0
0
0
0
0
0
0
0
0
-16
-67
6
127
0
0
4
0

Can anyone explain why it outputs those other numbers when my stdin stops after I input 1 2 3 ctrl+D and how I can stop that from happening? I want my array to be the size of how many numbers I input, but I am also having trouble with that if anyone has hints!

Thanks!

bummydummy
  • 13
  • 4
  • 3
    See also: [Why is “while( !feof(file) )” always wrong?](https://stackoverflow.com/q/5431941/2505965) – Oka Oct 03 '22 at 22:46
  • 1
    Not only is `while(!feof(stdin))` wrong, putting `if(feof(stdin)) break;` at the top of the loop is doubly wrong! – Steve Summit Oct 03 '22 at 22:54
  • Oh I see, my mistake. This is what I get for not understanding what a segment of a code really means. Thank you guys!! – bummydummy Oct 03 '22 at 23:13
  • 1
    `result = strcmp(argv[1], number);` You will get nasty results if you do not supply any arguments to this program... Check. Do not presume that argv[1] is not NULL... User input is what breaks programs that do not protect themselves. – Fe2O3 Oct 03 '22 at 23:33
  • One more thing: `scanf()` is a bad habit that is the root of many SO questions. Learn to use `fgets()` instead. Far more capable and 'safe'. – Fe2O3 Oct 04 '22 at 00:14

1 Answers1

0

The innermost loop is not necessary, only one loop is enough. When converting with scanf, check its return value. Also, only print the number of elements actually converted. Fixing these, a first corrected version is:

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

int main(int argc, char* argv[])
{   
    int count = 0;
    char* number = "-n";
    int result;
    int numArray[50];

    int i = 0;

    if (argc != 2)
        return EXIT_FAILURE;

    result = strcmp(argv[1], number);
    if (result == 0) {
        printf("Numbers Only\n");
        while (1) { 

          int number = 0;
          int ret = scanf("%d", &number);
          if (ret == 1)
            numArray[i++] = number;

          if (i == 50 || ret < 1)
            break;
        }
    }
    for (int j=0; j< i; j++){
        printf("%d\n", numArray[j]);
    }
}   

Testing:

$ gcc main.c && ./a.out -n  
Numbers Only
1   
2   
3   
<CTRL + d here>
1   
2   
3

Edits from comments: Removed while (!feof(stdin)), expanded error checking to avoid infinite loop and added a check before accessing argv.

Jardel Lucca
  • 1,115
  • 3
  • 10
  • 19
  • 1
    If, despite the instruction to enter "Numbers Only", the user types a letter, the code goes into an infinite loop. Suggest changing `i == 50 || ret == EOF` to `i == 50 || ret < 1`. – Steve Summit Oct 03 '22 at 23:04
  • @SteveSummit certainly the code can be improved, but infinite loops for such simple use case is definitely too bad. Thanks for the suggestion! – Jardel Lucca Oct 03 '22 at 23:16
  • 1
    I almost forgot to change the for loop at the bottom and was so worried why I was still getting extra numbers! Thank you, this definitely worked for me and was very helpful in achieving my goal. Instead of the i==50, I include ret < 1 alone (mainly because I plan on changing the size). – bummydummy Oct 03 '22 at 23:34
  • 1
    Glad you've added code to ensure argv[1] is not used if argc == 1... I'll remove my previous comment. Another line of thought is that answers should never suggest UB is not an issue... – Fe2O3 Oct 04 '22 at 03:08
  • 1
    @Fe2O3 I'll take more care next time to at least double check very basic errors such as UB. Thinking better that should not be accepted regardless. Thanks for reviewing the answer! – Jardel Lucca Oct 04 '22 at 03:25