From: Timo Sirainen <timo.sirainen@dovecot.fi>
Date: Mon, 16 May 2016 13:09:52 +0000 (+0300)
Subject: global: Don't assume o_stream_send_istream() return bytes sent.
X-Git-Tag: 2.3.0.rc1~3726
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f339a8e73beea7684ea634941ea82593dea522ee;p=thirdparty%2Fdovecot%2Fcore.git

global: Don't assume o_stream_send_istream() return bytes sent.

This won't be true soon after its API changes.
---

diff --git a/src/imap/cmd-getmetadata.c b/src/imap/cmd-getmetadata.c
index 676304f428..03187813ff 100644
--- a/src/imap/cmd-getmetadata.c
+++ b/src/imap/cmd-getmetadata.c
@@ -22,7 +22,7 @@ struct imap_getmetadata_context {
 	unsigned int depth;
 
 	struct istream *cur_stream;
-	uoff_t cur_stream_offset, cur_stream_size;
+	uoff_t cur_stream_size;
 
 	struct imap_metadata_iter *iter;
 	string_t *iter_entry_prefix;
@@ -203,7 +203,7 @@ static void cmd_getmetadata_send_entry(struct imap_getmetadata_context *ctx,
 		str_printfa(str, " ~{%"PRIuUOFF_T"}\r\n", value_len);
 		o_stream_nsend(client->output, str_data(str), str_len(str));
 
-		ctx->cur_stream_offset = 0;
+		i_assert(value.value_stream->v_offset == 0);
 		ctx->cur_stream_size = value_len;
 		ctx->cur_stream = value.value_stream;
 	}
@@ -212,19 +212,10 @@ static void cmd_getmetadata_send_entry(struct imap_getmetadata_context *ctx,
 static bool
 cmd_getmetadata_stream_continue(struct imap_getmetadata_context *ctx)
 {
-	off_t ret;
-
 	o_stream_set_max_buffer_size(ctx->cmd->client->output, 0);
-	ret = o_stream_send_istream(ctx->cmd->client->output, ctx->cur_stream);
+	(void)o_stream_send_istream(ctx->cmd->client->output, ctx->cur_stream);
 	o_stream_set_max_buffer_size(ctx->cmd->client->output, (size_t)-1);
 
-	if (ret > 0)
-		ctx->cur_stream_offset += ret;
-
-	if (ctx->cur_stream_offset == ctx->cur_stream_size) {
-		/* finished */
-		return TRUE;
-	}
 	if (ctx->cur_stream->stream_errno != 0) {
 		i_error("read(%s) failed: %s",
 			i_stream_get_name(ctx->cur_stream),
@@ -233,6 +224,10 @@ cmd_getmetadata_stream_continue(struct imap_getmetadata_context *ctx)
 				  "Internal GETMETADATA failure");
 		return TRUE;
 	}
+	if (ctx->cur_stream->v_offset == ctx->cur_stream_size) {
+		/* finished */
+		return TRUE;
+	}
 	if (!i_stream_have_bytes_left(ctx->cur_stream)) {
 		/* Input stream gave less data than expected */
 		i_error("read(%s): GETMETADATA stream had less data than expected",
diff --git a/src/imap/imap-fetch-body.c b/src/imap/imap-fetch-body.c
index f434f2c894..87d1875226 100644
--- a/src/imap/imap-fetch-body.c
+++ b/src/imap/imap-fetch-body.c
@@ -92,19 +92,19 @@ static int fetch_stream_continue(struct imap_fetch_context *ctx)
 {
 	struct imap_fetch_state *state = &ctx->state;
 	const char *disconnect_reason;
+	uoff_t orig_input_offset = state->cur_input->v_offset;
 	off_t ret;
 
 	o_stream_set_max_buffer_size(ctx->client->output, 0);
 	ret = o_stream_send_istream(ctx->client->output, state->cur_input);
 	o_stream_set_max_buffer_size(ctx->client->output, (size_t)-1);
 
-	if (ret > 0) {
-		state->cur_offset += ret;
-		if (ctx->state.cur_stats_sizep != NULL)
-			*ctx->state.cur_stats_sizep += ret;
+	if (ctx->state.cur_stats_sizep != NULL) {
+		*ctx->state.cur_stats_sizep +=
+			state->cur_input->v_offset - orig_input_offset;
 	}
 
-	if (state->cur_offset != state->cur_size) {
+	if (state->cur_input->v_offset != state->cur_size) {
 		/* unfinished */
 		if (state->cur_input->stream_errno != 0) {
 			fetch_read_error(ctx, &disconnect_reason);
@@ -119,7 +119,7 @@ static int fetch_stream_continue(struct imap_fetch_context *ctx)
 				"%"PRIuUOFF_T" vs %"PRIuUOFF_T,
 				i_stream_get_name(state->cur_input),
 				state->cur_human_name,
-				state->cur_offset, state->cur_size));
+				state->cur_input->v_offset, state->cur_size));
 			client_disconnect(ctx->client, "FETCH failed");
 			return -1;
 		}
@@ -184,6 +184,7 @@ static int fetch_body_msgpart(struct imap_fetch_context *ctx, struct mail *mail,
 
 	if (imap_msgpart_open(mail, body->msgpart, &result) < 0)
 		return -1;
+	i_assert(result.input->v_offset == 0);
 	ctx->state.cur_input = result.input;
 	ctx->state.cur_size = result.size;
 	ctx->state.cur_size_field = result.size_field;
@@ -475,6 +476,7 @@ fetch_and_free_msgpart(struct imap_fetch_context *ctx,
 	imap_msgpart_free(_msgpart);
 	if (ret < 0)
 		return -1;
+	i_assert(result.input->v_offset == 0);
 	ctx->state.cur_input = result.input;
 	ctx->state.cur_size = result.size;
 	ctx->state.cur_size_field = result.size_field;
diff --git a/src/imap/imap-fetch.c b/src/imap/imap-fetch.c
index 82c4647fbd..ebac6a5f5e 100644
--- a/src/imap/imap-fetch.c
+++ b/src/imap/imap-fetch.c
@@ -457,7 +457,6 @@ static int imap_fetch_more_int(struct imap_fetch_context *ctx, bool cancel)
 		}
 
 		state->cont_handler = NULL;
-		state->cur_offset = 0;
                 state->cur_handler++;
 		if (state->cur_input != NULL)
 			i_stream_unref(&state->cur_input);
@@ -523,7 +522,6 @@ static int imap_fetch_more_int(struct imap_fetch_context *ctx, bool cancel)
 			}
 
 			state->cont_handler = NULL;
-			state->cur_offset = 0;
 			if (state->cur_input != NULL)
 				i_stream_unref(&state->cur_input);
 		}
diff --git a/src/imap/imap-fetch.h b/src/imap/imap-fetch.h
index aa37d779bb..e2d12e836a 100644
--- a/src/imap/imap-fetch.h
+++ b/src/imap/imap-fetch.h
@@ -53,7 +53,7 @@ struct imap_fetch_state {
 	struct mail *cur_mail;
 	unsigned int cur_handler;
 	const char *cur_human_name;
-	uoff_t cur_size, cur_offset;
+	uoff_t cur_size;
 	enum mail_fetch_field cur_size_field;
 	string_t *cur_str;
 	struct istream *cur_input;
diff --git a/src/lib-fs/fs-api.c b/src/lib-fs/fs-api.c
index 4f6a5ea625..c6f2ff4098 100644
--- a/src/lib-fs/fs-api.c
+++ b/src/lib-fs/fs-api.c
@@ -831,7 +831,7 @@ int fs_default_copy(struct fs_file *src, struct fs_file *dest)
 		dest->copy_input = fs_read_stream(src, IO_BLOCK_SIZE);
 		dest->copy_output = fs_write_stream(dest);
 	}
-	while (o_stream_send_istream(dest->copy_output, dest->copy_input) > 0) ;
+	(void)o_stream_send_istream(dest->copy_output, dest->copy_input);
 	if (dest->copy_input->stream_errno != 0) {
 		errno = dest->copy_input->stream_errno;
 		fs_set_error(dest->fs, "read(%s) failed: %s",
diff --git a/src/lib-http/test-http-payload.c b/src/lib-http/test-http-payload.c
index 6d0320adb1..e3dba78e27 100644
--- a/src/lib-http/test-http-payload.c
+++ b/src/lib-http/test-http-payload.c
@@ -239,7 +239,7 @@ client_handle_download_request(
 
 	if (blocking) {
 		output = http_server_response_get_payload_output(resp, TRUE);
-		while ((ret=o_stream_send_istream	(output, fstream)) > 0);
+		ret=o_stream_send_istream(output, fstream);
 		if (ret < 0) {
 			i_fatal("test server: download: "
 				"failed to send blocking file payload");
@@ -353,8 +353,7 @@ client_handle_echo_request(struct client_request *creq,
 			payload_input = partial;
 		}
 
-		while ((ret=o_stream_send_istream
-			(payload_output, payload_input)) > 0);
+		ret = o_stream_send_istream(payload_output, payload_input);
 		if (ret < 0) {
 			i_fatal("test server: echo: "
 				"failed to receive blocking echo payload");
@@ -372,8 +371,7 @@ client_handle_echo_request(struct client_request *creq,
 		http_server_response_add_header(resp, "Content-Type", "text/plain");
 
 		payload_output = http_server_response_get_payload_output(resp, TRUE);
-		while ((ret=o_stream_send_istream
-			(payload_output, payload_input)) > 0);
+		ret = o_stream_send_istream(payload_output, payload_input);
 		if (ret < 0) {
 			i_fatal("test server: echo: "
 				"failed to send blocking echo payload");
diff --git a/src/lib-storage/index/dbox-common/dbox-file-fix.c b/src/lib-storage/index/dbox-common/dbox-file-fix.c
index 0140c085c8..49bc44511d 100644
--- a/src/lib-storage/index/dbox-common/dbox-file-fix.c
+++ b/src/lib-storage/index/dbox-common/dbox-file-fix.c
@@ -233,11 +233,10 @@ stream_copy(struct dbox_file *file, struct ostream *output,
 	    const char *out_path, uoff_t count)
 {
 	struct istream *input;
-	off_t bytes;
 	int ret = 0;
 
 	input = i_stream_create_limit(file->input, count);
-	bytes = o_stream_send_istream(output, input);
+	(void)o_stream_send_istream(output, input);
 
 	if (input->stream_errno != 0) {
 		mail_storage_set_critical(&file->storage->storage,
@@ -249,12 +248,11 @@ stream_copy(struct dbox_file *file, struct ostream *output,
 			"write(%s) failed: %s", out_path,
 			o_stream_get_error(output));
 		ret = -1;
-	} else if ((uoff_t)bytes != count) {
-		i_assert(bytes >= 0);
+	} else if (input->v_offset != count) {
 		mail_storage_set_critical(&file->storage->storage,
 			"o_stream_send_istream(%s) copied only %"
 			PRIuUOFF_T" of %"PRIuUOFF_T" bytes",
-			out_path, bytes, count);
+			out_path, input->v_offset, count);
 		ret = -1;
 	}
 	i_stream_unref(&input);
diff --git a/src/lib-storage/index/dbox-multi/mdbox-purge.c b/src/lib-storage/index/dbox-multi/mdbox-purge.c
index fc566a8430..ffafdc562e 100644
--- a/src/lib-storage/index/dbox-multi/mdbox-purge.c
+++ b/src/lib-storage/index/dbox-multi/mdbox-purge.c
@@ -191,7 +191,7 @@ mdbox_purge_save_msg(struct mdbox_purge_context *ctx, struct dbox_file *file,
 	struct ostream *output;
 	enum mdbox_map_append_flags append_flags;
 	uoff_t msg_size;
-	off_t ret;
+	int ret;
 
 	if (ctx->append_ctx == NULL)
 		ctx->append_ctx = mdbox_map_append_begin(ctx->atomic);
@@ -206,36 +206,37 @@ mdbox_purge_save_msg(struct mdbox_purge_context *ctx, struct dbox_file *file,
 	i_assert(file != out_file_append->file);
 
 	input = i_stream_create_limit(file->input, msg_size);
-	ret = o_stream_send_istream(output, input);
+	(void)o_stream_send_istream(output, input);
 	if (input->stream_errno != 0) {
 		mail_storage_set_critical(&file->storage->storage,
 			"read(%s) failed: %s", file->cur_path,
 			i_stream_get_error(input));
-		i_stream_unref(&input);
-		return -1;
-	}
-	i_stream_unref(&input);
-	if (o_stream_nfinish(output) < 0) {
+		ret = -1;
+	} else if (o_stream_nfinish(output) < 0) {
 		mail_storage_set_critical(&file->storage->storage,
 					  "write(%s) failed: %s",
 					  out_file_append->file->cur_path,
 					  o_stream_get_error(output));
-		return -1;
-	}
-	if (ret != (off_t)msg_size) {
-		i_assert(ret < (off_t)msg_size);
+		ret = -1;
+	} else if (input->v_offset != msg_size) {
+		i_assert(input->v_offset < msg_size);
 		i_assert(i_stream_is_eof(file->input));
 
 		dbox_file_set_corrupted(file, "truncated message at EOF");
-		return 0;
+		ret = 0;
+	} else {
+		ret = 1;
 	}
+	i_stream_unref(&input);
 
-	/* copy metadata */
-	if ((ret = mdbox_file_metadata_copy(file, output)) <= 0)
-		return ret;
+	if (ret > 0) {
+		/* copy metadata */
+		if ((ret = mdbox_file_metadata_copy(file, output)) <= 0)
+			return ret;
 
-	mdbox_map_append_finish(ctx->append_ctx);
-	return 1;
+		mdbox_map_append_finish(ctx->append_ctx);
+	}
+	return ret;
 }
 
 static int
diff --git a/src/lib-storage/index/mbox/mbox-sync-rewrite.c b/src/lib-storage/index/mbox/mbox-sync-rewrite.c
index ef902441a7..9e12377c91 100644
--- a/src/lib-storage/index/mbox/mbox-sync-rewrite.c
+++ b/src/lib-storage/index/mbox/mbox-sync-rewrite.c
@@ -15,9 +15,10 @@
 int mbox_move(struct mbox_sync_context *sync_ctx,
 	      uoff_t dest, uoff_t source, uoff_t size)
 {
+	struct mbox_mailbox *mbox = sync_ctx->mbox;
 	struct istream *input;
 	struct ostream *output;
-	off_t ret;
+	int ret;
 
 	i_assert(source > 0 || (dest != 1 && dest != 2));
 	i_assert(size < OFF_T_MAX);
@@ -36,27 +37,34 @@ int mbox_move(struct mbox_sync_context *sync_ctx,
 		return -1;
 	}
 
+	/* we're moving data within a file. it really shouldn't be failing at
+	   this point or we're corrupted. */
 	input = i_stream_create_limit(sync_ctx->file_input, size);
-	ret = o_stream_send_istream(output, input);
-	i_stream_unref(&input);
-
-        if (ret == (off_t)size)
-		ret = 0;
-	else if (ret >= 0) {
+	(void)o_stream_send_istream(output, input);
+	if (input->stream_errno != 0) {
+		mail_storage_set_critical(&mbox->storage->storage,
+			"read() failed with mbox file %s: %s",
+			mailbox_get_path(&mbox->box), i_stream_get_error(input));
+		ret = -1;
+	} else if (output->stream_errno != 0) {
+		mail_storage_set_critical(&mbox->storage->storage,
+			"write() failed with mbox file %s: %s",
+			mailbox_get_path(&mbox->box), o_stream_get_error(output));
+		ret = -1;
+	} else if (input->v_offset != size) {
 		mbox_sync_set_critical(sync_ctx,
 			"mbox_move(%"PRIuUOFF_T", %"PRIuUOFF_T", %"PRIuUOFF_T
 			") moved only %"PRIuUOFF_T" bytes",
-			dest, source, size, (uoff_t)ret);
+			dest, source, size, input->v_offset);
 		ret = -1;
-	} else if (ret < 0) {
-		errno = output->stream_errno;
-		mbox_set_syscall_error(sync_ctx->mbox,
-				       "o_stream_send_istream()");
+	} else {
+		ret = 0;
 	}
+	i_stream_unref(&input);
 
 	mbox_sync_file_updated(sync_ctx, FALSE);
 	o_stream_destroy(&output);
-	return (int)ret;
+	return ret;
 }
 
 static int mbox_fill_space(struct mbox_sync_context *sync_ctx,
diff --git a/src/lib/file-copy.c b/src/lib/file-copy.c
index 2931fcd3a6..fdba4607d2 100644
--- a/src/lib/file-copy.c
+++ b/src/lib/file-copy.c
@@ -70,8 +70,7 @@ static int file_copy_to_tmp(const char *srcpath, const char *tmppath,
 	input = i_stream_create_fd(fd_in, IO_BLOCK_SIZE, FALSE);
 	output = o_stream_create_fd_file(fd_out, 0, FALSE);
 
-	while ((ret = o_stream_send_istream(output, input)) > 0) ;
-
+	ret = o_stream_send_istream(output, input);
 	if (ret < 0)
 		i_error("write(%s) failed: %m", tmppath);