Implementing getchar with read

Question

I was trying to implement getchar() function using read() in unistd.h.

Since system calls are pricy, I wanted to execute less read() functions as possible.

If I use "getchar", it works fine. However, "mygetchar" does not work in this case.

Can anyone point out what I have done wrong below?

#include <stdio.h>
#include <unistd.h>

#define BUF_SIZE 1024

int startIndex;
int endIndex;

int mygetchar(void){
  char buffer[BUF_SIZE];
  startIndex=0;
  endIndex=0;
  if(startIndex == endIndex){
    int r;
    r = read(0,buffer,BUF_SIZE);
    startIndex=0;
    endIndex=r;
  }
  return buffer[startIndex++];
}


int main(){
  char c;
  int i=0;
  do{
    c = mygetchar();
    putchar(c);
    i++;
  }
  while(c != EOF);
  return 0;
}

You need to make `buffer` a global as well. Otherwise you're reading and discarding up to 1023 characters per read call, and subsequent calls to `mygetchar` return garbage. — Anthony, Mar 20 '13 at 02:15

score 1 · Answer 1 · edited May 23 '17 at 12:07

1

Think carefully about your buffer. What happens to the buffer when the function call ends? It goes away.

This means that for 1023 out of 1024 calls, your buffer is unitialized and your offsets are pointing into nonsensical data.

Basically you need a global variable for the buffer too:

static char buf[BUF_SIZE];
static size_t bufCur = 0;
static size_t bufEnd = 0;

int mygetchar(void)
{
    // ...
}

(Note that the static is pretty much pointless when your code is all in one file. If you were to pull your mygetchar into a header and implementation file though, you would want to use a static global so as to keep it from being linkable from outside of the same compilation unit.)

(Fun fact: the 0s for bufCur and bufEnd actually can be left implicit. For clarity, I would put them, but the standard dictates that they are to be zero-initialized.

As Jonathan Leffler pointed out, unless you plan on using the buffer elsewhere (and I don't know where that would be), there's no need for a global. You can just use a static variable inside of the function:

void mygetchar(void)
{
    static buf[BUF_SIZE];
    static size_t bufCur = 0;
    static size_t bufEnd = 0;
    // ...
}

edited May 23 '17 at 12:07

Community

1
1

answered Mar 20 '13 at 02:15

Corbin

33,060
6
68
78

2

Well, he's actually calling `read` once every 1024 calls (provided there is enough in the file to fill up the buffer in one call). Every subsequent call is returning an index into an uninitialised array, because `buffer` is a local variable. – Anthony Mar 20 '13 at 02:17
@anthony-arnold Ah, you're right. I didn't notice that his positions were global :). – Corbin Mar 20 '13 at 02:18
3

Or the three variables could all be made `static` in the function. That might be better; it would prevent any other code from accessing the variables. – Jonathan Leffler Mar 20 '13 at 02:27
@JonathanLeffler True. For some reason I assumed he would be using the buffer elsewhere, but I can't think of any other functions that would need it, and better to keep the scope as tight as possible. I shall edit in a second. – Corbin Mar 20 '13 at 02:31

Implementing getchar with read

1 Answers1