lib-fs: Log previous fs_file error if it's being overridden

This makes sure that errors aren't lost. If fs_file_last_error() has been
used it means that the error has been looked at and there's no need to log
it.

Some of the "expected" errors won't be logged though. This check is done
based on errno. This is also why this commit reorders
fs_file_set_error_async() to set errno=EAGAIN earlier.

diff --git a/src/lib-fs/fs-api-private.h b/src/lib-fs/fs-api-private.h
index bfee8136d55e37e110f3d72a464d5ed309998645..e541534731a856ffbccb87b0d0e5b745dbbcda9d 100644 (file)
--- a/src/lib-fs/fs-api-private.h
+++ b/src/lib-fs/fs-api-private.h
@@ -136,6 +136,7 @@ struct fs_file {
        bool lookup_metadata_counted:1;
        bool stat_counted:1;
        bool istream_open:1;
+       bool last_error_changed:1;
 };
 
 struct fs_lock {

diff --git a/src/lib-fs/fs-api.c b/src/lib-fs/fs-api.c
index 8fcd135fb9b9d8af1bb983237fb1fa4b659ae519..ec8d9bd7d41978dd057305d9f6713b94a0a39773 100644 (file)
--- a/src/lib-fs/fs-api.c
+++ b/src/lib-fs/fs-api.c
@@ -551,8 +551,26 @@ fs_set_verror(struct event *event, const char *fmt, va_list args)
 
        /* free old error after strdup in case args point to the old error */
        if (file != NULL) {
+               char *old_error = file->last_error;
                file = fs_file_get_error_file(file);
 
+               if (old_error != NULL && strcmp(old_error, new_error) == 0) {
+                       /* identical error - ignore */
+               } else if (file->last_error_changed) {
+                       /* multiple fs_set_error() calls used without
+                          fs_file_last_error() in the middle. */
+                       e_error(file->event, "%s (overwriting error)", old_error);
+               }
+               if (errno == EAGAIN || errno == ENOENT || errno == EEXIST ||
+                   errno == ENOTEMPTY) {
+                       /* These are (or can be) expected errors - don't log
+                          them if they have a missing fs_file_last_error()
+                          call */
+                       file->last_error_changed = FALSE;
+               } else {
+                       file->last_error_changed = TRUE;
+               }
+
                i_free(file->last_error);
                file->last_error = new_error;
        } else {
@@ -570,6 +588,7 @@ const char *fs_file_last_error(struct fs_file *file)
 {
        struct fs_file *error_file = fs_file_get_error_file(file);
 
+       error_file->last_error_changed = FALSE;
        if (error_file->last_error == NULL)
                return "BUG: Unknown file error";
        return error_file->last_error;
@@ -857,6 +876,9 @@ void fs_write_stream_abort_error(struct fs_file *file, struct ostream **output,
        va_list args;
        va_start(args, error_fmt);
        fs_set_verror(file->event, error_fmt, args);
+       /* the error shouldn't be automatically logged if
+          fs_file_last_error() is no longer used */
+       fs_file_get_error_file(file)->last_error_changed = FALSE;
        fs_write_stream_abort(file, output);
        va_end(args);
 }
@@ -1255,8 +1277,8 @@ void fs_set_error(struct event *event, const char *fmt, ...)
 
 void fs_file_set_error_async(struct fs_file *file)
 {
-       fs_set_error(file->event, "Asynchronous operation in progress");
        errno = EAGAIN;
+       fs_set_error(file->event, "Asynchronous operation in progress");
 }
 
 static uint64_t