Custom get line input function

Question

I am reading over the K&R book, and am a little stuck.

What is wrong with the following?

void getInput(int* output) {
   int c, i;
   for(i=0; (c = getchar()) != '\n'; i++)
     output[i] = c; // printf("%c", c) prints the c value as expected
   output[++i] = '\0';
}

When I run the program it never gets out of the loop and I have to Ctrl+C to exit. However if I replace the fifth line with printf("%c", c);, it prints out all the input just fine after hitting enter and creating the new line.

Answer 1

What is wrong with the following?

1. void getInput(int* output) {

Why is the input argument an int* when what you want to store in an array of characters? Probably

void getInput(char* output) {

is better.

Also, how do you know that the output pointer is pointing somewhere where you hold enough memory to write the user's input? Maybe you must have the maximum buffer length as an extra parameter to avoid buffer overflow errors as PW pointed out.

5.   output[++i] = '\0';

i has already been incremented an extra time inside the for loop, so you can just do:

output[i] = '\0';

Other than these, the program runs fine and outputs what we input until return.

FWIW, I tested it by calling it like so:

 int main(void)
{
    char o[100];
    getInput(o);
    printf("%s", o);
    return 0;
}

Answer 2

It looks correct to me as written except that you don't need to increment i outside of the loop. The i gets incremented right before the loop exits, thus it is already where you want it.

Make sure that a '\n' is actually making it into c.

Sometimes an '\n' will get thrown away as a delimiter.

Answer 3

your last code as posted have 3 errors I can see:

char* userInput[MAX_INPUT_SIZE];

Should be:

char userInput[MAX_INPUT_SIZE+1];

(this was already mentioned by Pax Diablo)

getInput(&userInput);

Should be:

getInput( userInput );

This last error means you passed to getInput an address inside your call stack. you have a memory overwrite. probably one of your calls to getchar() returnes to the wrong address.

Answer 4

a simple way to risk buffer overflow, because output's size is never passed/checked

Answer 5

Have you tried using a debugger? You should step through the code in gdb or visual studio or whatever you are using and see what is going on. You said you were a beginner so maybe you hadn't considered that yet - this is a pretty normal debugging technique to use.

Answer 6

Here is the complete program with a couple of updates from your input, but it still won't make it out of the loop. BTW this was exercise 1-24 on pg 34

#include <stdio.h>

#define STACK_SIZE 50
#define MAX_INPUT_SIZE 1000
#define FALSE 0
#define TRUE 1

void getInput();
int validInput();

int main() {
  char* userInput[MAX_INPUT_SIZE];

  getInput(&userInput);

  if (validInput(&userInput) == TRUE)
    printf("Compile complete");
  else
    printf("Error");
}

// Functions
void getInput(char* output) {
  int c, i;
  for(i=0; (c = getchar()) != '\n' && c != EOF && i <= MAX_INPUT_SIZE; i++)
    output[i] = c;
  output[i] = '\0';
}

int validInput(char* input) {
  char stack[STACK_SIZE];
  int c;
  int j;

  for (j=0; (c = input[j]) != '\0'; ) {
    switch(c){
      case '[': case '(': case '{':
        stack[j++] = c;
        break;
      case ']': case ')': case '}':
        if (c == ']' && stack[j] != '[')
          return FALSE;
        else if (c == '}' && stack[j] != '{')
          return FALSE;
        else if (c == ')' && stack[j] != '(')
          return FALSE;

        // decrement the stack's index  
        --j;
        break;
    }
  }

  return TRUE;
}

Answer 7

Here is the final working code. I must say I picked up quite a bit from doing this one. Thanks for the help and pointers.

Any suggestions on how things could be done better?

#include <stdio.h>

#define STACK_SIZE 50
#define MAX_INPUT_SIZE 1000
#define FALSE 0
#define TRUE !FALSE

void get_input();
int valid_input();

int main() {
  char user_input[MAX_INPUT_SIZE + 1]; // +1 for the \0

  get_input(user_input);

  if (valid_input(user_input))
    printf("Success\n");
  else
    printf("Error\n");
}

// Functions
void get_input(char* output) {
  int c, i;
  for(i=0; (c = getchar()) != '\n' && c != EOF && i <= MAX_INPUT_SIZE; i++)
    output[i] = c;
  output[i] = '\0';
}

int valid_input(char* input) {
  char stack[STACK_SIZE];
  char c;
  int i = 0;
  int stack_index = -1;

  while ((c = input[i]) != '\0' && i < STACK_SIZE) {
    switch(c){
      case '[': case '(': case '{':
        stack_index++; 
        stack[stack_index] = c;
        break;
      case ']': case ')': case '}':
        if ((c == ']' && stack[stack_index] != '[') ||
            (c == '}' && stack[stack_index] != '{') ||
            (c == ')' && stack[stack_index] != '('))
          return FALSE;

        // decrement the stack's index now that the closing bracket is found  
        stack_index--;
        break;
    }
    i++;
  }

  // stack index should be back where it started
  return (stack_index == -1);
}