CODEWITHSUNDEEP
cparsing
John Allison
John Allison

Reputation: 367

C Parser repeating outputs unusually

Alright, bear with me because there's some weird stuff here. I'm currently trying to implement a SUPER simple shell with max 20 char commands and no piping, etc. I'm also learning C along with it so please forgive any egregious errors or usage.

Here's my code for a parser:

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

int main(void) {
    int running = 1;

    /* Main loop */
    while(running==1) {
        // The max size of the input is 20 bytes
        char input[20];
        // Read string from user
        fgets(input, sizeof(input), stdin);

        /* ----- PARSE INPUT ----- */
        char buffer[20];
        for(int i = 0; i < sizeof(input); i++) {
            buffer[i] = input[i];
            /* ------ CHECK BUFFER AGAINST VARIOUS COMMANDS ----- */
            // TALK TO THE HAND
            if (strcmp(buffer, "TALK TO THE HAND") == 0) {
                // execute TALK TO THE HAND
                printf("TALK TO THE HAND\n");
            }
            else {
                printf("WHAT DID I DO WRONG?\n");
            }
        }
    }
}

My expected output when inputting TALK TO THE HAND is just it echoing that, but what I get is this:

TALK TO THE HAND
WHAT DID I DO WRONG?
WHAT DID I DO WRONG?
WHAT DID I DO WRONG?
WHAT DID I DO WRONG?
WHAT DID I DO WRONG?
WHAT DID I DO WRONG?
WHAT DID I DO WRONG?
WHAT DID I DO WRONG?
WHAT DID I DO WRONG?
WHAT DID I DO WRONG?
WHAT DID I DO WRONG?
WHAT DID I DO WRONG?
WHAT DID I DO WRONG?
WHAT DID I DO WRONG?
WHAT DID I DO WRONG?
TALK TO THE HAND
WHAT DID I DO WRONG?
WHAT DID I DO WRONG?
WHAT DID I DO WRONG?
WHAT DID I DO WRONG?

and I have no idea why it seemingly loops over and over again with different outputs.

Upvotes: 0

Views: 45

Answers (2)

Evgeniy
Evgeniy

Reputation: 302

sizeof is not a function that returning length of char-array. This is the keyword that returning a count of bytes which needed to allocation into memory any types and structures. Use strlen instead.

Try to print sizeof(char) and you will understand

Upvotes: 0

chux
chux

Reputation: 153338

Code iterates too far

Iterate to the length of the string, not the size of the buffer. 

    // for(int i = 0; i < sizeof(input); i++) {
    for(int i = 0; input[i]; i++) {

In any case, this iteration is not needed. Just compare to input

//if (strcmp(buffer, "TALK TO THE HAND") == 0) {
if (strcmp(input, "TALK TO THE HAND") == 0) {

If code wants to copy a string:

strcpy(buffer, input);

Be certain buffer is adequate in size.

Compares not accounting for '\n' that was read.

Lop off potential '\n'

fgets(input, sizeof(input), stdin);
// add
input[strcspn(input, "\n")] = '\0';

Buffer too small

buffer[] needs to hold 20 characters, '\n' and '\0'.

// char input[20];
char input[20 + 1 + 1];

Advanced

fgets() could return NULL which indicates stdin is closed.

User may enter too much text, good to detect that

#define INPUT_N 20
//                extra  \n  \0
char input[INPUT_N + 1 + 1 + 1]; // I'd just go with INPUT_N*2

if (fgets(input, sizeof(input), stdin) == NULL) {
  fprintf(stderr, "Input closed\n");
  return EXIT_FAILURE;
}

size_t len = strlen(input);
if (len > 0 && input[len-1] == '\n') {
  input[--len] = '\0';  // lop off \n
}

if (len > INPUT_N) {
  // exit or otherwise cope with too much data
  fprintf(stderr, "Excessive long input\n");
  return EXIT_FAILURE;
}

...

Upvotes: 2

Related Questions