> I noticed that some of the temporary error conditions will exit the
> function. You might want to consider using continues in the while
> form.
Thanks for pointing this out.
If the IRC server is configured for low load situations, I'd
prefer not rejecting connections and thus return from the
function. For the remaining two checks, it makes sense to
use continue. I've expanded the comments where appropiate.
> I'm afraid I won't accept anything that uses goto except in
> error exit conditions in functions that need to do cleanups due to
> stylistic prejudices.
Yes, therefore the submission of two patches (-no-goto).
> I have a slight concern about
> indentation, so you might want to look at that, but it's relatively
> minor, as I could correct it when I commit.
That would be appreciated. I found no style guide and have
thus relied on vim with tabstop=4,shiftwidth=2,expandtab.
- Sascha Experience IRCG
http://schumann.cx/ http://schumann.cx/ircg
Index: listener.c
===================================================================
RCS file: /home/coder-com/cvs/ircu2.10/ircd/listener.c,v
retrieving revision 1.17.2.1
diff -u -r1.17.2.1 listener.c
--- listener.c 2002/01/11 00:20:32 1.17.2.1
+++ listener.c 2002/05/17 17:30:02
@@ -442,48 +442,70 @@
* point, just assume that connections cannot
* be accepted until some old is closed first.
*/
- if (-1 == (fd = accept(listener->fd, (struct sockaddr*) &addr,
- &addrlen))) {
- /* Lotsa admins seem to have problems with not giving enough file
- * descriptors to their server so we'll add a generic warning mechanism
- * here. If it turns out too many messages are generated for
- * meaningless reasons we can filter them back.
+
+ /*
+ * This piece of code implements multi-accept, based
+ * on the idea that poll/select can only be efficient,
+ * if we succeed in handling all available events,
+ * i.e. accept all pending connections.
+ *
+ * http://www.hpl.hp.com/techreports/2000/HPL-2000-174.html
+ */
+
+ while (1) {
+ if (-1 == (fd = accept(listener->fd, (struct sockaddr*) &addr,
+ &addrlen))) {
+
+ /* There is no other connection pending */
+ if (errno == EAGAIN || errno == EWOULDBLOCK)
+ return;
+
+ /* Lotsa admins seem to have problems with not giving enough file
+ * descriptors to their server so we'll add a generic warning mechanism
+ * here. If it turns out too many messages are generated for
+ * meaningless reasons we can filter them back.
+ */
+ sendto_opmask_butone(0, SNO_TCPCOMMON,
+ "Unable to accept connection: %m");
+ return;
+ }
+ /*
+ * check for connection limit. If this fd exceeds the limit,
+ * all further accept()ed connections will also exceed it.
+ * Enable the server to clear out other connections before
+ * continuing to accept() new connections.
*/
- sendto_opmask_butone(0, SNO_TCPCOMMON,
- "Unable to accept connection: %m");
- return;
- }
- /*
- * check for connection limit
- */
- if (fd > MAXCLIENTS - 1) {
- ++ServerStats->is_ref;
- send(fd, "ERROR :All connections in use\r\n", 32, 0);
- close(fd);
- return;
- }
- /*
- * check to see if listener is shutting down
- */
- if (!listener->active) {
- ++ServerStats->is_ref;
- send(fd, "ERROR :Use another port\r\n", 25, 0);
- close(fd);
- return;
- }
- /*
- * check to see if connection is allowed for this address mask
- */
- if (!connection_allowed((const char*) &addr,
- (const char*) &listener->mask)) {
- ++ServerStats->is_ref;
- send(fd, "ERROR :Use another port\r\n", 25, 0);
- close(fd);
- return;
- }
- ++ServerStats->is_ac;
-/* nextping = CurrentTime; */
+ if (fd > MAXCLIENTS - 1) {
+ ++ServerStats->is_ref;
+ send(fd, "ERROR :All connections in use\r\n", 32, 0);
+ close(fd);
+ return;
+ }
+ /*
+ * check to see if listener is shutting down. Continue
+ * to accept(), because it makes sense to clear our the
+ * socket's queue as fast as possible.
+ */
+ if (!listener->active) {
+ ++ServerStats->is_ref;
+ send(fd, "ERROR :Use another port\r\n", 25, 0);
+ close(fd);
+ continue;
+ }
+ /*
+ * check to see if connection is allowed for this address mask
+ */
+ if (!connection_allowed((const char*) &addr,
+ (const char*) &listener->mask)) {
+ ++ServerStats->is_ref;
+ send(fd, "ERROR :Use another port\r\n", 25, 0);
+ close(fd);
+ continue;
+ }
+ ++ServerStats->is_ac;
+ /* nextping = CurrentTime; */
- add_connection(listener, fd);
+ add_connection(listener, fd);
+ }
}
}
