Factor out parts of client_read() into client_socket_read() and

client_get_reply(), downsizing it from 170+ to just over 30 lines.
The gotos are now gone, too.

2 files changed, 170 insertions, 144 deletions

diff --git a/usr.sbin/smtpd/client.c b/usr.sbin/smtpd/client.c
index 735d98706ad..a1589f97705 100644
--- a/usr.sbin/smtpd/client.c
+++ b/usr.sbin/smtpd/client.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: client.c,v 1.24 2010/01/02 11:06:37 jacekm Exp $	*/
+/*	$OpenBSD: client.c,v 1.25 2010/01/02 13:42:42 jacekm Exp $	*/
 
 /*
  * Copyright (c) 2009 Jacek Masiulaniec <jacekm@dobremiasto.net>
@@ -39,15 +39,21 @@
 #include "client.h"
 
 struct client_cmd *cmd_new(int, char *, ...);
+void		 cmd_free(struct client_cmd *);
 int		 client_read(struct smtp_client *);
+void		 client_get_reply(struct smtp_client *, struct client_cmd *,
+		     int *);
 int		 client_write(struct smtp_client *);
 int		 client_use_extensions(struct smtp_client *);
 void		 client_status(struct smtp_client *, char *, ...);
-
 int		 client_getln(struct smtp_client *, int);
 void		 client_putln(struct smtp_client *, char *, ...);
 struct buf	*client_content_read(FILE *, size_t);
 int		 client_poll(struct smtp_client *);
+void		 client_quit(struct smtp_client *);
+
+int		 client_socket_read(struct smtp_client *);
+int		 client_socket_write(struct smtp_client *);
 
 #ifndef CLIENT_NO_SSL
 int		 client_ssl_connect(struct smtp_client *);
@@ -149,6 +155,13 @@ cmd_new(int type, char *fmt, ...)
 	return (cmd);
 }
 
+void
+cmd_free(struct client_cmd *cmd)
+{
+	free(cmd->action);
+	free(cmd);
+}
+
 /*
  * Request that connection be secured using SSL from the start.
  */
@@ -334,101 +347,77 @@ client_talk(struct smtp_client *sp, int writable)
 int
 client_read(struct smtp_client *sp)
 {
-	struct client_cmd	*cmd, *c;
-	int			 type;
-	void			*data;
-
-#ifndef CLIENT_NO_SSL
-	if (sp->ssl) {
-		switch (ssl_buf_read(sp->ssl, &sp->r)) {
-		case SSL_ERROR_NONE:
-			break;
-
-		case SSL_ERROR_WANT_READ:
-			return (CLIENT_STOP_WRITE);
+	struct client_cmd	*cmd;
+	int			 ret;
 
-		case SSL_ERROR_WANT_WRITE:
-			return (CLIENT_WANT_WRITE);
+	if ((ret = client_socket_read(sp)))
+		return (ret);
 
-		default:
-			client_status(sp, "130 ssl_buf_read error");
-			return (CLIENT_DONE);
-		}
-	}
-#endif
-	if (sp->ssl == NULL) {
-		errno = 0;
-		if (buf_read(sp->w.fd, &sp->r) == -1) {
-			if (errno)
-				client_status(sp, "130 buf_read: %s",
-				    strerror(errno));
-			else
-				client_status(sp, "130 buf_read: "
-				    "connection closed");
+	while ((cmd = TAILQ_FIRST(&sp->cmdrecvq))) {
+		if (client_getln(sp, cmd->type) < 0)
 			return (CLIENT_DONE);
-		}
-	}
+		if (*sp->reply == '\0')
+			return client_poll(sp);
 
-again:
-	/* each reply corresponds to past command */
-	if ((cmd = TAILQ_FIRST(&sp->cmdrecvq)) == NULL) {
-		client_status(sp, "170 client_read: unexpected data");
-		return (CLIENT_DONE);
-	}
+		/* reply fully received */
+		TAILQ_REMOVE(&sp->cmdrecvq, cmd, entry);
+		sp->cmdi--;
 
-	/* get server reply */
-	if (client_getln(sp, cmd->type) < 0)
-		goto quit2;
-	if (*sp->reply == '\0')
-		return client_poll(sp);
+		/* if dying, ignore all replies as we wait for an EOF. */
+		if (!sp->dying)
+			client_get_reply(sp, cmd, &ret);
 
-	/* reply fully received */
-	TAILQ_REMOVE(&sp->cmdrecvq, cmd, entry);
-	type = cmd->type;
-	data = cmd->data;
-	free(cmd->action);
-	free(cmd);
-	sp->cmdi--;
+		cmd_free(cmd);
 
-	/* dying?  ignore all replies as we wait for EOF. */
-	if (sp->dying)
-		return client_poll(sp);
+		/* handle custom return code, e.g. CLIENT_RCPT_FAIL */
+		if (ret)
+			return (ret);
+	}
+
+	return client_poll(sp);
+}
 
-	switch (type) {
+/*
+ * Parse reply to previously sent command.
+ */
+void
+client_get_reply(struct smtp_client *sp, struct client_cmd *cmd, int *ret)
+{
+	switch (cmd->type) {
 	case CLIENT_BANNER:
-		if (*sp->reply != '2')
-			goto quit;
-		break;
+	case CLIENT_HELO:
+	case CLIENT_MAILFROM:
+		if (*sp->reply != '2') {
+			client_status(sp, "%s", sp->reply);
+			client_quit(sp);
+		}
+		return;
 
 	case CLIENT_EHLO:
 		if (*sp->reply != '2') {
 			if (sp->exts[CLIENT_EXT_STARTTLS].must ||
-			    sp->exts[CLIENT_EXT_AUTH].must)
-				goto quit;
-			else {
-				c = cmd_new(CLIENT_HELO, "HELO %s", sp->ehlo);
-				TAILQ_INSERT_HEAD(&sp->cmdsendq, c, entry);
+			    sp->exts[CLIENT_EXT_AUTH].must) {
+				client_status(sp, "%s", sp->reply);
+				client_quit(sp);
+			} else {
+				cmd = cmd_new(CLIENT_HELO, "HELO %s", sp->ehlo);
+				TAILQ_INSERT_HEAD(&sp->cmdsendq, cmd, entry);
 			}
-			break;
+			return;
 		}
 
 		if (client_use_extensions(sp) < 0)
-			goto quit2;
-		break;
-
-	case CLIENT_HELO:
-		if (*sp->reply != '2')
-			goto quit;
-		break;
+			client_quit(sp);
+		return;
 
 	case CLIENT_STARTTLS:
 		if (*sp->reply != '2') {
 			sp->exts[CLIENT_EXT_STARTTLS].fail = 1;
 			if (client_use_extensions(sp) < 0)
-				goto quit2;
+				client_quit(sp);
 		} else
 			sp->ssl_handshake = 1;
-		break;
+		return;
 
 	case CLIENT_AUTH:
 		if (*sp->reply != '2')
@@ -437,27 +426,23 @@ again:
 			sp->exts[CLIENT_EXT_AUTH].done = 1;
 
 		if (client_use_extensions(sp) < 0)
-			goto quit2;
-		break;
-
-	case CLIENT_MAILFROM:
-		if (*sp->reply != '2')
-			goto quit;
-		break;
+			client_quit(sp);
+		return;
 
 	case CLIENT_RCPTTO:
 		if (*sp->reply == '2')
 			sp->rcptokay++;
 		else {
-			sp->rcptfail = data;
-			return (CLIENT_RCPT_FAIL);
+			sp->rcptfail = cmd->data;
+			*ret = CLIENT_RCPT_FAIL;
 		}
-		break;
+		return;
 
 	case CLIENT_DATA:
-		if (*sp->reply != '3')
-			goto quit;
-		else if (sp->rcptokay > 0) {
+		if (*sp->reply != '3') {
+			client_status(sp, "%s", sp->reply);
+			client_quit(sp);
+		} else if (sp->rcptokay > 0) {
 			sp->content = sp->head;
 			sp->head = NULL;
 			if (sp->content == NULL)
@@ -470,36 +455,16 @@ again:
 			 */
 			client_status(sp, "600 all recipients refused");
 		}
-		break;
+		return;
 
 	case CLIENT_DOT:
-		goto quit;
+		client_status(sp, "%s", sp->reply);
+		client_quit(sp);
+		return;
 
 	default:
-		fatalx("client_read: unexpected type");
-	}
-
-	if (!TAILQ_EMPTY(&sp->cmdrecvq))
-		goto again;
-
-	return client_poll(sp);
-
-quit:
-	client_status(sp, "%s", sp->reply);
-quit2:
-	/* reduce send queue to just the final QUIT */
-	while ((c = TAILQ_FIRST(&sp->cmdsendq))) {
-		if (c->type == CLIENT_QUIT)
-			break;
-		TAILQ_REMOVE(&sp->cmdsendq, c, entry);
-		free(c->action);
-		free(c);
+		fatalx("client_get_reply: unexpected type");
 	}
-
-	/* ignore all replies from now on, eg. those still in the pipeline */
-	sp->dying = 1; 
-
-	return client_poll(sp);
 }
 
 /*
@@ -509,6 +474,7 @@ int
 client_write(struct smtp_client *sp)
 {
 	struct client_cmd	*cmd;
+	int			 ret;
 
 	if (sp->content) {
 		buf_close(&sp->w, sp->content);
@@ -546,30 +512,8 @@ client_write(struct smtp_client *sp)
 		}
 	}
 
-#ifndef CLIENT_NO_SSL
-	if (sp->ssl) {
-		switch (ssl_buf_write(sp->ssl, &sp->w)) {
-		case SSL_ERROR_NONE:
-			break;
-
-		case SSL_ERROR_WANT_READ:
-			return (CLIENT_STOP_WRITE);
-
-		case SSL_ERROR_WANT_WRITE:
-			return (CLIENT_WANT_WRITE);
-
-		default:
-			client_status(sp, "130 ssl_buf_write error");
-			return (CLIENT_DONE);
-		}
-	}
-#endif
-	if (sp->ssl == NULL) {
-		if (buf_write(&sp->w) < 0) {
-			client_status(sp, "130 buf_write error");
-			return (CLIENT_DONE);
-		}
-	}
+	if ((ret = client_socket_write(sp)))
+		return (ret);
 
 	return client_poll(sp);
 }
@@ -645,13 +589,11 @@ client_close(struct smtp_client *sp)
 	msgbuf_clear(&sp->w);
 	while ((cmd = TAILQ_FIRST(&sp->cmdsendq))) {
 		TAILQ_REMOVE(&sp->cmdsendq, cmd, entry);
-		free(cmd->action);
-		free(cmd);
+		cmd_free(cmd);
 	}
 	while ((cmd = TAILQ_FIRST(&sp->cmdrecvq))) {
 		TAILQ_REMOVE(&sp->cmdrecvq, cmd, entry);
-		free(cmd->action);
-		free(cmd);
+		cmd_free(cmd);
 	}
 #ifndef CLIENT_NO_SSL
 	if (sp->ssl)
@@ -878,6 +820,90 @@ client_poll(struct smtp_client *sp)
 }
 
 /*
+ * Move to dying stage.
+ */
+void
+client_quit(struct smtp_client *sp)
+{
+	struct client_cmd *cmd;
+
+	while ((cmd = TAILQ_FIRST(&sp->cmdsendq))) {
+		if (cmd->type == CLIENT_QUIT)
+			break;
+		TAILQ_REMOVE(&sp->cmdsendq, cmd, entry);
+		cmd_free(cmd);
+	}
+	sp->dying = 1; 
+}
+
+/*
+ * Receive data from socket to internal buffer.
+ */
+int
+client_socket_read(struct smtp_client *sp)
+{
+#ifndef CLIENT_NO_SSL
+	if (sp->ssl) {
+		switch (ssl_buf_read(sp->ssl, &sp->r)) {
+		case SSL_ERROR_NONE:
+			break;
+		case SSL_ERROR_WANT_READ:
+			return (CLIENT_STOP_WRITE);
+		case SSL_ERROR_WANT_WRITE:
+			return (CLIENT_WANT_WRITE);
+		default:
+			client_status(sp, "130 ssl_buf_read error");
+			return (CLIENT_DONE);
+		}
+	}
+#endif
+	if (sp->ssl == NULL) {
+		errno = 0;
+		if (buf_read(sp->w.fd, &sp->r) == -1) {
+			if (errno)
+				client_status(sp, "130 buf_read: %s",
+				    strerror(errno));
+			else
+				client_status(sp, "130 buf_read: "
+				    "connection closed");
+			return (CLIENT_DONE);
+		}
+	}
+	return (0);
+}
+
+/*
+ * Send data to socket from the msgbuf.
+ */
+int
+client_socket_write(struct smtp_client *sp)
+{
+#ifndef CLIENT_NO_SSL
+	if (sp->ssl) {
+		switch (ssl_buf_write(sp->ssl, &sp->w)) {
+		case SSL_ERROR_NONE:
+			break;
+		case SSL_ERROR_WANT_READ:
+			return (CLIENT_STOP_WRITE);
+		case SSL_ERROR_WANT_WRITE:
+			return (CLIENT_WANT_WRITE);
+		default:
+			client_status(sp, "130 ssl_buf_write error");
+			return (CLIENT_DONE);
+		}
+	}
+#endif
+	if (sp->ssl == NULL) {
+		if (buf_write(&sp->w) < 0) {
+			client_status(sp, "130 buf_write error");
+			return (CLIENT_DONE);
+		}
+	}
+
+	return (0);
+}
+
+/*
  * Read a full line from the read buffer.
  */
 char *
diff --git a/usr.sbin/smtpd/client.h b/usr.sbin/smtpd/client.h
index aa501fc01dd..e10458b3afe 100644
--- a/usr.sbin/smtpd/client.h
+++ b/usr.sbin/smtpd/client.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: client.h,v 1.9 2010/01/02 11:06:37 jacekm Exp $	*/
+/*	$OpenBSD: client.h,v 1.10 2010/01/02 13:42:42 jacekm Exp $	*/
 
 /*
  * Copyright (c) 2009 Jacek Masiulaniec <jacekm@dobremiasto.net>
@@ -23,10 +23,10 @@
 struct smtp_client;
 
 /* return codes for io routines */
-#define CLIENT_DONE		 0	/* finished */
-#define CLIENT_WANT_WRITE	-1	/* want read + write */
-#define CLIENT_STOP_WRITE	-2	/* want read */
-#define CLIENT_RCPT_FAIL	-3	/* recipient refused */
+#define CLIENT_DONE		-1	/* finished */
+#define CLIENT_WANT_WRITE	-2	/* want read + write */
+#define CLIENT_STOP_WRITE	-3	/* want read */
+#define CLIENT_RCPT_FAIL	-4	/* recipient refused */
 
 /* client commands */
 #define CLIENT_BANNER		0x1