CODEWITHSUNDEEP
c
Peter Hwang
Peter Hwang

Reputation: 991

Implementing getchar with read

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

Upvotes: 1

Views: 1172

Answers (1)

Corbin
Corbin

Reputation: 33447

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

Upvotes: 1

Related Questions