[LCP]sockets problem

Greg Black gjb at gbch.net
Wed Jan 30 07:01:06 UTC 2002


Paul Gearon wrote lots of good stuff, but broke one rule here:

| Next you make a big error by not checking the return value of the read() call.
| After the first time through the loop the data you've just read may not end with
| a zero, and you'll end up with left over data at the end of the buffer (at least
| the last byte of the buffer will always be 0, so you won't segfault).  You have
| a spare variable called "len" which I assume is for this purpose.
| 
| A better way to do this might be:
| 
|     if(FD_ISSET(scklisten,&clientset)) {
|       if (-1 != (sockno=accept(scklisten, (struct sockaddr *)&serv, &size))) {
|         len = read(sockno,buffer,34);
|         buffer[len] = 0;  /* make sure the string is null terminated! */
|         printf("\n%s\n",buffer);
|         close(sockno);
|       } else printf("Error accepting connection\n");
|     }

Although unlikely, it's possible that len will have a negative
value after the read(), so it should not be used as an array
index without checking.  The snippet in question should be more
like this:

    if ((len = read(sockno, buffer, 34)) < 0)
        scream_and_die();
    buffer[len] = 0;  /* make sure the string is null terminated! */

I didn't really check the whole thing, because it was a pretty
good discussion of the issues -- but this one jumped out at me.

Greg



More information about the linuxCprogramming mailing list