Introduce ast_careful_fwrite() and use in AMI to prevent partial writes.

This patch introduces a function to do careful writes on a file stream which
will handle timeouts and partial writes.  It is currently used in AMI to
address the issue that has been reported.  However, there are probably a few
other places where this could be used.

(closes issue #13546)
Reported by: srt
Tested by: russell
http://reviewboard.digium.com/r/104/


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@166282 65c4cc65-6c06-0410-ace0-fbb531ad65f3

include/asterisk/utils.h

@@ -326,6 +326,25 @@ int ast_wait_for_input(int fd, int ms);
 */
 int ast_carefulwrite(int fd, char *s, int len, int timeoutms);
 
+/*!
+ * \brief Write data to a file stream with a timeout
+ *
+ * \param f the file stream to write to
+ * \param fd the file description to poll on to know when the file stream can
+ *        be written to without blocking.
+ * \param s the buffer to write from
+ * \param len the number of bytes to write
+ * \param timeoutms The maximum amount of time to block in this function trying
+ *        to write, specified in milliseconds.
+ *
+ * \note This function assumes that the associated file stream has been set up
+ *       as non-blocking.
+ *
+ * \retval 0 success
+ * \retval -1 error
+ */
+int ast_careful_fwrite(FILE *f, int fd, const char *s, size_t len, int timeoutms);
+
 /*
  * Thread management support (should be moved to lock.h or a different header)
  */

main/manager.c

@@ -899,30 +899,7 @@ struct ast_variable *astman_get_variables(const struct message *m)
  */
 static int send_string(struct mansession *s, char *string)
 {
-	int len = strlen(string);	/* residual length */
+	return ast_careful_fwrite(s->f, s->fd, string, strlen(string), s->writetimeout);
-	char *src = string;
-	struct timeval start = ast_tvnow();
-	int n = 0;
-
-	for (;;) {
-		int elapsed;
-		struct pollfd fd;
-		n = fwrite(src, 1, len, s->f);	/* try to write the string, non blocking */
-		if (n == len /* ok */ || n < 0 /* error */)
-			break;
-		len -= n;	/* skip already written data */
-		src += n;
-		fd.fd = s->fd;
-		fd.events = POLLOUT;
-		n = -1;		/* error marker */
-		elapsed = ast_tvdiff_ms(ast_tvnow(), start);
-		if (elapsed > s->writetimeout)
-			break;
-		if (poll(&fd, 1, s->writetimeout - elapsed) < 1)
-			break;
-	}
-	fflush(s->f);
-	return n < 0 ? -1 : 0;
 }
 
 /*!

main/utils.c

@@ -1057,6 +1057,38 @@ int ast_wait_for_input(int fd, int ms)
 	return poll(pfd, 1, ms);
 }
 
+static int ast_wait_for_output(int fd, int timeoutms)
+{
+	struct pollfd pfd = {
+		.fd = fd,
+		.events = POLLOUT,
+	};
+	int res;
+
+	/* poll() until the fd is writable without blocking */
+	while ((res = poll(&pfd, 1, timeoutms)) <= 0) {
+		if (res == 0) {
+			/* timed out. */
+			ast_log(LOG_NOTICE, "Timed out trying to write\n");
+			return -1;
+		} else if (res == -1) {
+			/* poll() returned an error, check to see if it was fatal */
+
+			if (errno == EINTR || errno == EAGAIN) {
+				/* This was an acceptable error, go back into poll() */
+				continue;
+			}
+
+			/* Fatal error, bail. */
+			ast_log(LOG_ERROR, "poll returned error: %s\n", strerror(errno));
+
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
 /*!
  * Try to write string, but wait no more than ms milliseconds before timing out.
  *
@@ -1077,30 +1109,8 @@ int ast_carefulwrite(int fd, char *s, int len, int timeoutms)
 	int res = 0;
 
 	while (len) {
-		struct pollfd pfd = {
+		if (ast_wait_for_output(fd, timeoutms)) {
-			.fd = fd,
+			return -1;
-			.events = POLLOUT,
-		};
-
-		/* poll() until the fd is writable without blocking */
-		while ((res = poll(&pfd, 1, timeoutms)) <= 0) {
-			if (res == 0) {
-				/* timed out. */
-				ast_log(LOG_NOTICE, "Timed out trying to write\n");
-				return -1;
-			} else if (res == -1) {
-				/* poll() returned an error, check to see if it was fatal */
-
-				if (errno == EINTR || errno == EAGAIN) {
-					/* This was an acceptable error, go back into poll() */
-					continue;
-				}
-
-				/* Fatal error, bail. */
-				ast_log(LOG_ERROR, "poll returned error: %s\n", strerror(errno));
-
-				return -1;
-			}
 		}
 
 		res = write(fd, s, len);
@@ -1125,6 +1135,62 @@ int ast_carefulwrite(int fd, char *s, int len, int timeoutms)
 	return res;
 }
 
+int ast_careful_fwrite(FILE *f, int fd, const char *src, size_t len, int timeoutms)
+{
+	struct timeval start = ast_tvnow();
+	int n = 0;
+
+	while (len) {
+		int elapsed;
+
+		if (ast_wait_for_output(fd, timeoutms)) {
+			/* poll returned a fatal error, so bail out immediately. */
+			return -1;
+		}
+
+		/* Clear any errors from a previous write */
+		clearerr(f);
+
+		n = fwrite(src, 1, len, f);
+
+		if (ferror(f) && errno != EINTR && errno != EAGAIN) {
+			/* fatal error from fwrite() */
+			if (!feof(f)) {
+				/* Don't spam the logs if it was just that the connection is closed. */
+				ast_log(LOG_ERROR, "fwrite() returned error: %s\n", strerror(errno));
+			}
+			n = -1;
+			break;
+		}
+
+		/* Update for data already written to the socket */
+		len -= n;
+		src += n;
+
+		elapsed = ast_tvdiff_ms(ast_tvnow(), start);
+		if (elapsed > timeoutms) {
+			/* We've taken too long to write 
+			 * This is only an error condition if we haven't finished writing. */
+			n = len ? -1 : 0;
+			break;
+		}
+	}
+
+	while (fflush(f)) {
+		if (errno == EAGAIN || errno == EINTR) {
+			continue;
+		}
+		if (!feof(f)) {
+			/* Don't spam the logs if it was just that the connection is closed. */
+			ast_log(LOG_ERROR, "fflush() returned error: %s\n", strerror(errno));
+		}
+		n = -1;
+		break;
+	}
+
+	return n < 0 ? -1 : 0;
+}
+
 char *ast_strip_quoted(char *s, const char *beg_quotes, const char *end_quotes)
 {
 	char *e;