Why is this socket/ file descriptor assignment invalid?

Question

I'm trying to write a simple server in c that plays a two player game. It checks for incoming connections, and if there is no player1, it saves player1's file descriptor (to be used later for sending and receiving) and if there is no player2, it does the same. I have this loop set up that I modified from Here. My problem is that I want to receive from one, and send to the other, but it seems that my assignments are invalid. When I try to send to player2, it fails or it sends garbage. Sometimes, sending to player1 sends back to the server(?). Am I using select correctly and looping through the file descriptor set correctly? Any feedback would be appreciated.

// add the listener to the master set
FD_SET(listener, &master);

// keep track of the biggest file descriptor
fdmax = listener; // so far, it's this one

// main loop
while (1) {
    read_fds = master; // copy it
    if (select(fdmax+1, &read_fds, NULL, NULL, NULL) == -1) {
         error("select");
    }

    // run through the existing connections looking for data to read
    for(i = 0; i <= fdmax; i++) {

        //This indicates that someone is trying to do something
        if (FD_ISSET(i, &read_fds)) {
            if (i == listener) {

                addrlen = sizeof remoteaddr;
                newfd = accept(listener, (struct sockaddr *)&remoteaddr, &addrlen);

                if (newfd == -1) {
                    error("accept");
                } else {
                    FD_SET(newfd, &master);
                    if (newfd > fdmax) {
                        fdmax = newfd;
                    }

                    /* If we have the maximum number of players, we tell if that it's busy */
                    if (players >= 2) {
                         toobusy(fdmax); close(fdmax); FD_CLR(fdmax, &master);
                    }  else {                                                        
                         //Problem here?
                         if (player1_fd == -1) {
                               player1_fd = newfd;                                  
                         }

                         if ((player1_fd != -1) && (player2_fd == -1)) {
                               player2_fd = newfd;                                   
                         }

                         players++;
                         if (players == 2) {
                               sendhandles(); //says two players exist
                         }
                    }
                }
            } else {
                //Possible problems here
                if (i == player1_fd || i == player2_fd) {
                     receive(i); //Processes the messages
                }
            }
        }
    }
}

Answer 1

The toobusy part should use newfd, not fdmax. Otherwise there's no easy spotted error in this code.

Your comment "Sometimes, sending to player1 sends back to the server(?)" makes me think that player1_fd and player2_fd might be uninitialized or perhaps initialized to 0 instead of -1. You should double check that you set them to -1 before the loop.

A few additionally notes:

• Are you sure master is 0 initialized? Have you called FD_ZERO on it?
• You should use FD_COPY to copy master to read_fds.

Finally, I'd recommend to use a library for event handling, such as libevent or libev.