Skorpius
Skorpius

Reputation: 2255

Select in C- file descriptors may not be set up correctly?

So I've been going over Beej's networking guide. On an earlier question, I was told that to remove a lot of the freezing happening due to blocking I/O I needed to use select to check file descriptors and such before receiving so I only received when something was waiting in the socket- I tried to also have a program where the user can both send and receive- So I want to try to add select for stdin as well and use fgets if possible- I'm assuming It's just checking fd at 0...

Problem is, the current code I have isn't doing anything- Any ideas?

char prev[100];
    char nil[100];
    memset(nil, 0, sizeof nil);


    struct timeval tv;
    fd_set read_fds;
    tv.tv_sec = 0;
    tv.tv_usec = 500000;

    FD_ZERO(&read_fds);
    FD_SET(sockfd, &read_fds);
    FD_SET(STDIN, &read_fds);

    int fdmax = new_fd+1;



    while (1) {



/*      if (send(sockfd, "Howdy", 100, 0) == -1) {
            perror("send");
            exit(1);
        }*/


         if (select(fdmax, &read_fds, NULL, NULL, &tv) == -1) {
            perror("select");
            exit(4);
            }

            int i = 0;

            for (i = 0; i <= fdmax; i++) {

                if (FD_ISSET(i, &read_fds)) {



                    if (i == STDIN) {

                        printf("You have input");
                    }

                    if (i == sockfd) {



                        if ((numbytes = recv(sockfd, buf, 99, 0)) == -1) {
                            perror("recv");
                            exit(1);
                        }

                        if (strcmp(prev, buf) == 0 || strcmp(nil, buf) == 0 ) {
                            continue;
                        }

                        buf[numbytes] = '\0';

                        printf("\n%s\n", buf);

                        memmove(prev, buf, sizeof buf);
                    }
                }
            }

Upvotes: 0

Views: 53

Answers (2)

Jeremy Friesner
Jeremy Friesner

Reputation: 73081

All of the code in the block below should be inside your while() loop (i.e. just before each call to select()). select() modifies the state of those data structures before it returns, so you have to re-initialize them to their default-state before each call to select().

struct timeval tv;
fd_set read_fds;
tv.tv_sec = 0;
tv.tv_usec = 500000;

FD_ZERO(&read_fds);
FD_SET(sockfd, &read_fds);
FD_SET(STDIN, &read_fds);

Also, fdmax should be set to the maximum of all of the fd's you passed to FD_SET, plus 1:

int fdmax = std::max(sockfd, STDIN)+1;

Finally, your for loop isn't necessary; you could instead just do

if (FD_ISSET(STDIN, &read_fds)) {
   printf("You have input");
}
else if (FD_ISSET(sockfd, &read_fds)) {
   // code to recv() from sockfd goes here
}

Upvotes: 0

Halim Qarroum
Halim Qarroum

Reputation: 14103

That is because you are probably using select on a Linux system, and you might not have seen this part of its manual page :

On Linux, select() modifies timeout to reflect the amount of time not slept; most other implementations do not do this. (POSIX.1-2001 permits either behav‐ ior.) This causes problems both when Linux code which reads timeout is ported to other operating systems, and when code is ported to Linux that reuses a struct timeval for multiple select()s in a loop without reinitializing it. Consider timeout to be undefined after select() returns.

That means that the timeval structure passed to select is modified internally and you should reset it between calls to the select function.

As a side note, you might also want to look at epoll which helps you watch file descriptors change events like select does, but is more natural to use, and far more scalable.

Upvotes: 1

Related Questions