From: Timo Sirainen <timo.sirainen@open-xchange.com>
Date: Tue, 21 Jan 2020 13:59:51 +0000 (+0200)
Subject: lib-fs: Add errno parameter to fs_set_error() change callers to use it.
X-Git-Tag: 2.3.10~136
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=72439accd23a7186267911d50f65a5566fec8796;p=thirdparty%2Fdovecot%2Fcore.git

lib-fs: Add errno parameter to fs_set_error() change callers to use it.

This is now explicit so it can't be forgotten accidentally. Fixes several
bugs where errno wasn't set on error conditions, which could have caused
callers to handle the error wrong.
---

diff --git a/src/lib-fs/fs-api-private.h b/src/lib-fs/fs-api-private.h
index 28394bc37a..395e3f3ff4 100644
--- a/src/lib-fs/fs-api-private.h
+++ b/src/lib-fs/fs-api-private.h
@@ -168,8 +168,10 @@ extern const struct fs fs_class_test;
 
 void fs_class_register(const struct fs *fs_class);
 
-/* Event must be fs_file or fs_iter events */
-void fs_set_error(struct event *event, const char *fmt, ...) ATTR_FORMAT(2, 3);
+/* Event must be fs_file or fs_iter events. Set errno from err. */
+void fs_set_error(struct event *event, int err,
+		  const char *fmt, ...) ATTR_FORMAT(3, 4);
+/* Like fs_set_error(), but use the existing errno. */
 void fs_set_error_errno(struct event *event, const char *fmt, ...) ATTR_FORMAT(2, 3);
 void fs_file_set_error_async(struct fs_file *file);
 
diff --git a/src/lib-fs/fs-api.c b/src/lib-fs/fs-api.c
index 3ba76c6d22..8bdfa0fd4a 100644
--- a/src/lib-fs/fs-api.c
+++ b/src/lib-fs/fs-api.c
@@ -490,7 +490,7 @@ int fs_get_metadata(struct fs_file *file,
 			*metadata_r = &file->metadata;
 			return 0;
 		}
-		fs_set_error(file->event, "Metadata not supported by backend");
+		fs_set_error(file->event, ENOTSUP, "Metadata not supported by backend");
 		return -1;
 	}
 	if (!file->read_or_prefetch_counted &&
@@ -645,7 +645,9 @@ ssize_t fs_read_via_stream(struct fs_file *file, void *buf, size_t size)
 		return -1;
 	}
 	if (ret < 0 && file->pending_read_input->stream_errno != 0) {
-		fs_set_error(file->event, "read(%s) failed: %s",
+		fs_set_error(file->event,
+			     file->pending_read_input->stream_errno,
+			     "read(%s) failed: %s",
 			     i_stream_get_name(file->pending_read_input),
 			     i_stream_get_error(file->pending_read_input));
 	} else {
@@ -859,7 +861,8 @@ int fs_write_stream_finish(struct fs_file *file, struct ostream **output)
 		o_stream_uncork(file->output);
 		if ((ret = o_stream_finish(file->output)) <= 0) {
 			i_assert(ret < 0);
-			fs_set_error(file->event, "write(%s) failed: %s",
+			fs_set_error(file->event, file->output->stream_errno,
+				     "write(%s) failed: %s",
 				     o_stream_get_name(file->output),
 				     o_stream_get_error(file->output));
 			success = FALSE;
@@ -1006,7 +1009,7 @@ int fs_stat(struct fs_file *file, struct stat *st_r)
 	int ret;
 
 	if (file->fs->v.stat == NULL) {
-		fs_set_error(file->event, "fs_stat() not supported");
+		fs_set_error(file->event, ENOTSUP, "fs_stat() not supported");
 		return -1;
 	}
 
@@ -1112,7 +1115,7 @@ int fs_copy(struct fs_file *src, struct fs_file *dest)
 	i_assert(src->fs == dest->fs);
 
 	if (src->fs->v.copy == NULL) {
-		fs_set_error(src->event, "fs_copy() not supported");
+		fs_set_error(src->event, ENOTSUP, "fs_copy() not supported");
 		return -1;
 	}
 
@@ -1232,7 +1235,7 @@ int fs_iter_deinit(struct fs_iter **_iter, const char **error_r)
 	DLLIST_REMOVE(&fs->iters, iter);
 
 	if (fs->v.iter_deinit == NULL) {
-		fs_set_error(event, "FS iteration not supported");
+		fs_set_error(event, ENOTSUP, "FS iteration not supported");
 		ret = -1;
 	} else T_BEGIN {
 		ret = iter->fs->v.iter_deinit(iter);
@@ -1284,10 +1287,13 @@ const struct fs_stats *fs_get_stats(struct fs *fs)
 	return &fs->stats;
 }
 
-void fs_set_error(struct event *event, const char *fmt, ...)
+void fs_set_error(struct event *event, int err, const char *fmt, ...)
 {
 	va_list args;
 
+	i_assert(err != 0);
+
+	errno = err;
 	va_start(args, fmt);
 	fs_set_verror(event, fmt, args);
 	va_end(args);
@@ -1306,8 +1312,7 @@ void fs_set_error_errno(struct event *event, const char *fmt, ...)
 
 void fs_file_set_error_async(struct fs_file *file)
 {
-	errno = EAGAIN;
-	fs_set_error(file->event, "Asynchronous operation in progress");
+	fs_set_error(file->event, EAGAIN, "Asynchronous operation in progress");
 }
 
 static uint64_t
diff --git a/src/lib-fs/fs-dict.c b/src/lib-fs/fs-dict.c
index 1cb635387d..4e1398a3e5 100644
--- a/src/lib-fs/fs-dict.c
+++ b/src/lib-fs/fs-dict.c
@@ -160,12 +160,12 @@ static int fs_dict_lookup(struct dict_fs_file *file)
 	if (ret > 0)
 		return 0;
 	else if (ret < 0) {
-		errno = EIO;
-		fs_set_error(file->file.event, "dict_lookup(%s) failed: %s", file->key, error);
+		fs_set_error(file->file.event, EIO,
+			     "dict_lookup(%s) failed: %s", file->key, error);
 		return -1;
 	} else {
-		errno = ENOENT;
-		fs_set_error(file->file.event, "Dict key %s doesn't exist", file->key);
+		fs_set_error(file->file.event, ENOENT,
+			     "Dict key %s doesn't exist", file->key);
 		return -1;
 	}
 }
@@ -243,8 +243,8 @@ static int fs_dict_write_stream_finish(struct fs_file *_file, bool success)
 	}
 	}
 	if (dict_transaction_commit(&trans, &error) < 0) {
-		errno = EIO;
-		fs_set_error(_file->event, "Dict transaction commit failed: %s", error);
+		fs_set_error(_file->event, EIO,
+			     "Dict transaction commit failed: %s", error);
 		return -1;
 	}
 	return 1;
@@ -272,8 +272,8 @@ static int fs_dict_delete(struct fs_file *_file)
 	trans = dict_transaction_begin(fs->dict);
 	dict_unset(trans, file->key);
 	if (dict_transaction_commit(&trans, &error) < 0) {
-		errno = EIO;
-		fs_set_error(_file->event, "Dict transaction commit failed: %s", error);
+		fs_set_error(_file->event, EIO,
+			     "Dict transaction commit failed: %s", error);
 		return -1;
 	}
 	return 0;
@@ -316,7 +316,8 @@ static int fs_dict_iter_deinit(struct fs_iter *_iter)
 
 	ret = dict_iterate_deinit(&iter->dict_iter, &error);
 	if (ret < 0)
-		fs_set_error(_iter->event, "Dict iteration failed: %s", error);
+		fs_set_error(_iter->event, EIO,
+			     "Dict iteration failed: %s", error);
 	return ret;
 }
 
diff --git a/src/lib-fs/fs-metawrap.c b/src/lib-fs/fs-metawrap.c
index 37fb9af80c..55e227592e 100644
--- a/src/lib-fs/fs-metawrap.c
+++ b/src/lib-fs/fs-metawrap.c
@@ -188,7 +188,8 @@ fs_metawrap_get_metadata(struct fs_file *_file,
 			fs_wait_async(_file->fs);
 		}
 		if (ret == -1 && file->input->stream_errno != 0) {
-			fs_set_error(_file->event, "read(%s) failed: %s",
+			fs_set_error(_file->event, file->input->stream_errno,
+				     "read(%s) failed: %s",
 				     i_stream_get_name(file->input),
 				     i_stream_get_error(file->input));
 			return -1;
@@ -433,7 +434,7 @@ static int fs_metawrap_stat(struct fs_file *_file, struct stat *st_r)
 		if (fs_stat(_file->parent, st_r) < 0)
 			return -1;
 		if ((uoff_t)st_r->st_size < file->metadata_write_size) {
-			fs_set_error(_file->event,
+			fs_set_error(_file->event, EIO,
 				"Just-written %s shrank unexpectedly "
 				"(%"PRIuUOFF_T" < %"PRIuUOFF_T")",
 				fs_file_path(_file), st_r->st_size,
@@ -451,7 +452,8 @@ static int fs_metawrap_stat(struct fs_file *_file, struct stat *st_r)
 		i_stream_ref(input);
 	}
 	if ((ret = i_stream_get_size(input, TRUE, &input_size)) < 0) {
-		fs_set_error(_file->event, "i_stream_get_size(%s) failed: %s",
+		fs_set_error(_file->event, input->stream_errno,
+			     "i_stream_get_size(%s) failed: %s",
 			     fs_file_path(_file), i_stream_get_error(input));
 		i_stream_unref(&input);
 		return -1;
@@ -459,9 +461,8 @@ static int fs_metawrap_stat(struct fs_file *_file, struct stat *st_r)
 	i_stream_unref(&input);
 	if (ret == 0) {
 		/* we shouldn't get here */
-		fs_set_error(_file->event, "i_stream_get_size(%s) returned size as unknown",
+		fs_set_error(_file->event, EIO, "i_stream_get_size(%s) returned size as unknown",
 			     fs_file_path(_file));
-		errno = EIO;
 		return -1;
 	}
 
diff --git a/src/lib-fs/fs-posix.c b/src/lib-fs/fs-posix.c
index 56b24479f1..c5aacf384a 100644
--- a/src/lib-fs/fs-posix.c
+++ b/src/lib-fs/fs-posix.c
@@ -623,10 +623,9 @@ static int fs_posix_write(struct fs_file *_file, const void *data, size_t size)
 		return -1;
 	}
 	if ((size_t)ret != size) {
-		fs_set_error(_file->event,
+		fs_set_error(_file->event, ENOSPC,
 			     "write(%s) returned %"PRIuSIZE_T"/%"PRIuSIZE_T,
 			     file->full_path, (size_t)ret, size);
-		errno = ENOSPC;
 		return -1;
 	}
 	return 0;
@@ -697,8 +696,9 @@ fs_posix_lock(struct fs_file *_file, unsigned int secs, struct fs_lock **lock_r)
 	switch (fs->lock_method) {
 	case FS_POSIX_LOCK_METHOD_FLOCK:
 #ifndef HAVE_FLOCK
-		fs_set_error(_file->event, "flock() not supported by OS "
-			     "(for file %s)", file->full_path);
+		fs_set_error(_file->event, ENOTSUP,
+			     "flock() not supported by OS (for file %s)",
+			     file->full_path);
 #else
 		if (secs == 0) {
 			ret = file_try_lock(file->fd, file->full_path, F_WRLCK,
diff --git a/src/lib-fs/fs-randomfail.c b/src/lib-fs/fs-randomfail.c
index 182992d7e0..8a3d696de7 100644
--- a/src/lib-fs/fs-randomfail.c
+++ b/src/lib-fs/fs-randomfail.c
@@ -240,8 +240,7 @@ static bool fs_random_fail(struct fs *_fs, struct event *event,
 	if (fs->op_probability[op] == 0)
 		return FALSE;
 	if ((unsigned int)i_rand_limit(100 * divider) <= fs->op_probability[op]) {
-		errno = EIO;
-		fs_set_error(event, RANDOMFAIL_ERROR);
+		fs_set_error(event, EIO, RANDOMFAIL_ERROR);
 		return TRUE;
 	}
 	return FALSE;
@@ -506,8 +505,7 @@ static int fs_randomfail_iter_deinit(struct fs_iter *_iter)
 	if ((ret = fs_iter_deinit(&iter->super, &error)) < 0)
 		fs_set_error_errno(_iter->event, "%s", error);
 	if (iter->fail_pos == 1) {
-		fs_set_error(_iter->event, RANDOMFAIL_ERROR);
-		errno = EIO;
+		fs_set_error(_iter->event, EIO, RANDOMFAIL_ERROR);
 		ret = -1;
 	}
 	return ret;
diff --git a/src/lib-fs/fs-sis-common.c b/src/lib-fs/fs-sis-common.c
index f516ac3ce0..654c44ec56 100644
--- a/src/lib-fs/fs-sis-common.c
+++ b/src/lib-fs/fs-sis-common.c
@@ -22,7 +22,7 @@ int fs_sis_path_parse(struct fs_file *file, const char *path,
 	/* assume filename begins with "<hash>-" */
 	p = strchr(fname, '-');
 	if (p == NULL) {
-		fs_set_error(file->event, "open(%s) failed: "
+		fs_set_error(file->event, EINVAL, "open(%s) failed: "
 			     "Filenames must begin with '<hash>-'", path);
 		return -1;
 	}
diff --git a/src/lib-fs/fs-sis-queue.c b/src/lib-fs/fs-sis-queue.c
index f5f894d8b9..35ac314162 100644
--- a/src/lib-fs/fs-sis-queue.c
+++ b/src/lib-fs/fs-sis-queue.c
@@ -81,7 +81,7 @@ fs_sis_queue_file_init(struct fs_file *_file, const char *path,
 	file->fs = fs;
 
 	if (mode == FS_OPEN_MODE_APPEND)
-		fs_set_error(_file->event, "APPEND mode not supported");
+		fs_set_error(_file->event, ENOTSUP, "APPEND mode not supported");
 	else
 		file->file.parent = fs_file_init_parent(_file, path, mode | flags);
 }
diff --git a/src/lib-fs/fs-sis.c b/src/lib-fs/fs-sis.c
index c0979aa8e5..0d3d1cd5d4 100644
--- a/src/lib-fs/fs-sis.c
+++ b/src/lib-fs/fs-sis.c
@@ -93,7 +93,7 @@ fs_sis_file_init(struct fs_file *_file, const char *path,
 	file->fs = fs;
 	file->open_mode = mode;
 	if (mode == FS_OPEN_MODE_APPEND) {
-		fs_set_error(_file->event, "APPEND mode not supported");
+		fs_set_error(_file->event, ENOTSUP, "APPEND mode not supported");
 		return;
 	}