(Pardon the mobile review) On Apr 1, 2014 9:16 PM, David Bremner <david@tethera.net> wrote: > > It is useful to able to tell whether a dump completed successfully in > situtions where we don't have access to the return code. "Situations." This commit message doesn't seem very related to atomicity? > --- > notmuch-dump.c | 61 +++++++++++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 48 insertions(+), 13 deletions(-) > > diff --git a/notmuch-dump.c b/notmuch-dump.c > index 28342b7..05ed6b4 100644 > --- a/notmuch-dump.c > +++ b/notmuch-dump.c > @@ -129,30 +129,65 @@ notmuch_database_dump (notmuch_database_t *notmuch, > { > gzFile output; > const char *mode = gzip_output ? "w9" : "wT"; > + const char *name_for_error = output_file_name ? output_file_name : "stdout"; > > - int ret; > + char *tempname = NULL; > + int outfd = -1; > + > + int ret = -1; > + > + if (output_file_name) { > + tempname = talloc_asprintf (notmuch, "%s.XXXXXX", output_file_name); > + outfd = mkstemp (tempname); > + } else { > + outfd = fileno (stdout); > + } > > - if (output_file_name) > - output = gzopen (output_file_name, mode); > - else > - output = gzdopen (fileno (stdout), mode); > + if (outfd < 0) { > + fprintf (stderr, "Bad output file %s\n", name_for_error); > + goto DONE; > + } > + > + output = gzdopen (outfd, mode); > > if (output == NULL) { > fprintf (stderr, "Error opening %s for (gzip) writing: %s\n", > - output_file_name ? output_file_name : "stdout", strerror (errno)); > - return EXIT_FAILURE; > + name_for_error, strerror (errno)); > + goto DONE; > } > > ret = database_dump_file (notmuch, output, query_str, output_format); > + if (ret) goto DONE; > > - if (gzflush (output, Z_FINISH)) { > - fprintf (stderr, "Error flushing output: %s\n", > - gzerror (output, NULL)); > - return EXIT_FAILURE; > + ret = gzflush (output, Z_FINISH); > + if (ret) { > + fprintf (stderr, "Error flushing output: %s\n", gzerror (output, NULL)); > + goto DONE; > } > > - if (output_file_name) > - gzclose_w (output); > + ret = fdatasync (outfd); This may fail if outfd isn't a regular file. We probably should only do this if output_file_name. Or we could ignore EINVAL (but that may mask bugs). > + if (ret) { > + perror ("fdatasync"); perror's fine for quick hacks, but it would be better to give a real error message here. > + goto DONE; > + } > + > + if (output_file_name) { > + ret = gzclose_w (output); > + if (ret != Z_OK) { > + ret = EXIT_FAILURE; > + goto DONE; > + } > + > + ret = rename (tempname, output_file_name); > + if (ret) { > + perror ("rename"); > + goto DONE; > + } > + > + } > + DONE: > + if (ret != EXIT_SUCCESS && output_file_name) > + (void) unlink (tempname); We need to gzclose on the error paths to free zlib's internal resources. This is a problem if we handed stdout to gzdopen. We shouldn't write any more to stdout anyway in that case, but maybe it would be better to dup stdout? > > return ret; > } > -- > 1.9.0 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch