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.

Are you pressing the enter key??? And, show us the whole code, you may not be calling getInput properly. And, output should be a char*. — paxdiablo, Oct 22 '08 at 05:49
The ++ in output[++i] means you skip one entry in your array - which is normally an array of char rather than an array of int, as Pax Diablo pointed out. Other people noted that you're not checking for EOF and that you're not checking for buffer overflow. You should start learning good habits now. — Jonathan Leffler, Oct 22 '08 at 05:58
PAX: I am pressing the enter key :) Jonathon: That I am. C is easy to read over, but is slaps you upside the head when you try to do it, at least that is what I am finding. — chris, Oct 22 '08 at 06:14

score 7 · Accepted Answer · edited May 23 '17 at 12:12

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;
}

score 1 · Answer 2 · answered Oct 22 '08 at 05:39

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.

score 1 · Answer 3 · answered Oct 22 '08 at 07:07

1

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.

answered Oct 22 '08 at 07:07

Raz

1,932
2
18
23

That makes sense, otherwise I am passing the address of the pointer. – chris Oct 22 '08 at 13:35

score 0 · Answer 4 · answered Oct 22 '08 at 05:42

0

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

answered Oct 22 '08 at 05:42

PW.

3,727
32
35

I had that in before, but having an issue with the code it got stripped out to make it easier to find the issue. – chris Oct 22 '08 at 05:52

chris · Answer 5 · 2008-10-22T13:30:20.353

0

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;
}

edited Oct 22 '08 at 13:30

answered Oct 22 '08 at 06:00

chris

4,332
5
41
61

The markup parser is playing with your code, taking < as a sign of beginning of a html tag... I think putting it within a code block (selecting the code and pressing Ctrl-K) might help, though I'm not sure. Strangely, the behaviour doesn't seem reproducible, at least in my simplistic tests. – Sundar R Oct 22 '08 at 06:22
For a start, 'char* userInput[MAX_INPUT_SIZE];' should be 'char userInput[MAX_INPUT_SIZE+1];' - that's two changes, char array instead of char-pointer array and extra space for holding null. Funnily enough, the first problem masks the second as you're allocating 4 times as much memory as you need. – paxdiablo Oct 22 '08 at 06:28
That can't be the exact code you have, getInput takes a char *, but you are passing in &userInput - you would get a compile error – 1800 INFORMATION Oct 22 '08 at 06:51
Since I was having issues with the formatting I cut some of the code off. I have now posted all the code. It does compile on my machine using gcc 4.0.1 – chris Oct 22 '08 at 13:31

score 0 · Answer 6 · answered Oct 22 '08 at 06:52

0

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.

answered Oct 22 '08 at 06:52

1800 INFORMATION

131,367
29
160
239

Unfortunately, I am just writing this up with textmate. I will check out Eclipse later today and use the debugger. – chris Oct 22 '08 at 13:37

score 0 · Answer 7 · answered Oct 23 '08 at 04:09

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);
}

Custom get line input function

7 Answers7