CODEWITHSUNDEEP
csocketsbuffer
Daniel
Daniel

Reputation: 47

Issues with socket buffer reader

I've been given this code for an assignment, there supposed to be errors in it but I can't actually figure out what this function is supposed to do, never mind figure out if there's any issues with it...

I am guessing that it's supposed to read the buffer line by line, but I've never seen it done this way before

The buffer that is sent to the function is empty.

int read_line(int sock, char *buffer) {
    size_t length = 0;

    while (1) {
        char data;
        int result = recv(sock, &data, 1, 0);

        if ((result <= 0) || (data == EOF)){
            perror("Connection closed");
            exit(1);
        }

        buffer[length] = data;
        length++;

        if (length >= 2 && buffer[length-2] == '\r' && buffer[length-1] == '\n') {
            buffer[length-2] = '\0';
            return length;
        }
    }
}

Thanks in advance!

Upvotes: 0

Views: 130

Answers (1)

m.k
m.k

Reputation: 118

I'd say the purpose of this function is to read a line that ends with \r\n from socket stream and store it in a char array as a string, therefore the \0 (string termination character) placement.

Ok, so what's wrong with the code?

I'd start with the input parameter char *buffer - inside the function you do not know its size so you cannot check if it exceeds its size limit and it could lead to buffer overflow. So it would be better to send buffer length as a parameter and check with every received byte if it can be stored.

EOF - it is defined as -1 and in this case actually doesn't make any sense, because nothing will be setting your data variable to EOF. The only thing you need to look out for is the end of socket stream (recv documentation). And here is an example for EOFusage. Feel free to remove (data == EOF) from condition.

Let's say you are receiving everything regularly and you receive your last input and connection closes, so you enter this case:

if ((result <= 0) || (data == EOF)){
    perror("Connection closed");
    exit(1);
}

The problem here is that you won't process your last line and the program will just end. Although, I might be wrong here since I don't know when the connection is getting regularly shut down. And a minor note here, result that equals to 0 isn't considered as an error, but a regular connection shutdown (or a 0-byte datagram was received).

I hope I haven't missed anything.

Upvotes: 2

Related Questions