Creating readline() function, reading from a file descriptor

Question

I've got a task to create a readline() function that would have the exact same behavior as read():

ssize_t read(int fd, void *buf, size_t count); (man read)

So it should return -1 on error and number of bytes read on success. Also, if the function receives a string with multiple \n chars, it should return only the first line and store the rest for a future use.

I was scrolling through this article: Reading a line ending with a newline character from a file descriptor

But I do not think the author had the same purpose - that is to work just as read()

I came up with this function:

//global buffer
char gBUF[BUFSIZ];

int readline2(int fd, char* buf){

    static char* resultBuffer = gBUF;
    int ret, mv;
    char temp[256];
    char* t;

    bzero(temp, sizeof(temp));



    t = strchr(resultBuffer,'\n');
    if(t != NULL){
        mv = t-resultBuffer+1;
        strncpy(temp,resultBuffer, mv);
        resultBuffer = resultBuffer + mv;

        temp[strlen(temp)] = '\0';
        strcpy(buf,temp);
        bzero(temp, sizeof(temp));
        ret = read(fd,&temp,256);
        temp[ret]='\0';
        strcat(resultBuffer,temp);

        return mv;
    }
    ret = read(fd,&temp,256);
    temp[ret]='\0';
    t = strchr(temp,'\n');
    mv = t-temp+1;
    strncpy(buf,temp,mv);

    strcat(resultBuffer,temp+mv);
    return ret;

}    

and I mean, it works well, although I had some real struggles with pointers and copying addresses instead of values.

My question is, how to improve this? I still have a feeling there's something missing or that something could significantly improve it.

I guess I should put a while inside and check whether the read string actually has a \n char and only return when it does, right?

I also do not like the idea of a global buffer, maybe I should just create a class/struct, something like this:

struct {
char BUF[BUFSIZ];
char *p = BUF;
}dataStruct;

to store the buffer and a pointer to it, but I do not really know how I could use that properly

Thanks for any suggestions!

Reading max 256bytes is a purpose, that should be the max;

I could've probably added that this function is to be used after in a multiprocess/multithread client server project. The server reads these lines from clients and should know what input came first, in what order the lines came, etc. The buffer should essentially work as a queue.

Answer 1

As we've basically worked out in the comments: when there's already a thing, use the thing. To get a line from a file descriptor, you can

1. Call fdopen on your file descriptor to get a FILE*
2. Check for NULL, perror and exit if necessary
3. Call fgets
4. Check for NULL again
5. Repeat 3-4 as necessary
6. fclose your FILE*
7. Do not call close on your file descriptor

Answer 2

You've got a bit of a nightmare of lengths. You should be using explicit lengths for everything, and to heck with NUL termination.

Lets look at some cases:

    ret = read(fd,&temp,256);
    temp[ret]='\0';

Well, temp is of size 256. (And you should write sizeof(temp) instead of 256.) This means, if you read 256 bytes, you write a null into the 257th byte in the buffer, and smash memory.

    temp[strlen(temp)] = '\0';

This finds the first NUL in temp, by offset, and then overwrites it with a NUL. A useless statement. And you should instead know how many bytes you have in temp. Then use memchr instead of strchr, memcpy instead of strcpy and strcat, etc...

 int readline2(int fd, char* buf){

This is trying to reproduce the prototype of read(), but you forgot the buffer length altogether. This makes your function more comparable to gets() then fgets(). gets() is one of the large security holes in C. Don't be like gets().

Edit: One more thing. If you pass in an fd for a terminal, and it is in raw mode, you might get just a couple characters, not a complete line. This is sometimes known as a "short read". You need to keep reading until you get a line. For testing, this can be easily simulated by reading only a few bytes at a time.