[PATCH 03/18] insert: open Maildir tmp file
Mark Walters
markwalters1009 at gmail.com
Sun Nov 18 07:47:18 PST 2012
Hi
I know this has been around for a long time but it still applies so I
will try and review it. I have been using it somewhat and all seems to
work.
As an aside I have a postpone/resume implementation based on top
of it.
One general remark is that I think more comments could be helpful.
On Wed, 25 Jul 2012, Peter Wang <novalazy at gmail.com> wrote:
> Open a unique file in the Maildir tmp directory.
> The new message is not yet copied into the file.
> ---
> notmuch-insert.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 79 insertions(+), 1 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index 21424cf..f01a6f2 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -20,12 +20,86 @@
>
> #include "notmuch-client.h"
>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +
> +static notmuch_bool_t
> +safe_gethostname (char *hostname, size_t hostname_size)
> +{
> + if (gethostname (hostname, hostname_size) == -1) {
> + strncpy (hostname, "unknown", hostname_size);
> + }
> + hostname[hostname_size - 1] = '\0';
> +
> + return (strchr (hostname, '/') == NULL);
> +}
Here is an example: a comment saying "like gethostname but guarantees
that hostname is null terminated. Returns true unless hostname contains
a /.
Also I found hostname_size a confusing term as it the size of the buffer
which is allowed to contain hostname rather than the size of
hostname. Maybe hostname_buffer_size or just len?
> +
> +static int
> +maildir_open_tmp (void *ctx, const char *dir, char **tmppath, char **newpath)
> +{
> + pid_t pid;
> + char hostname[256];
> + struct timeval tv;
> + char *filename;
> + int fd = -1;
> +
> + /* This is the file name format used by Dovecot. */
> + pid = getpid ();
> + if (! safe_gethostname (hostname, sizeof (hostname))) {
> + fprintf (stderr, "Error: invalid host name.\n");
> + return -1;
> + }
> + gettimeofday (&tv, NULL);
> + filename = talloc_asprintf (ctx, "%ld.M%ldP%d.%s",
> + tv.tv_sec, tv.tv_usec, pid, hostname);
> +
> + *tmppath = talloc_asprintf (ctx, "%s/tmp/%s", dir, filename);
Does there need to be rather more error checking after talloc
operations? Eg what if this allocation fails?
> +
> + do {
> + fd = open (*tmppath, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0666);
> + } while (fd == -1 && errno == EEXIST);
Am I misunderstanding or does this deadlock if the file *tmppath exists?
Do you want to recalculate tmppath inside the do while loop?
Also is 0666 standard or would 0644 or 0600 be better?
Best wishes
Mark
> +
> + if (fd != -1) {
> + *newpath = talloc_asprintf (ctx, "%s/new/%s", dir, filename);
> + }
> + else {
> + fprintf (stderr, "Error: opening %s: %s\n",
> + *tmppath, strerror (errno));
> + talloc_free (*tmppath);
> + }
> +
> + talloc_free (filename);
> +
> + return fd;
> +}
> +
> +static notmuch_bool_t
> +insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
> + const char *dir)
> +{
> + char *tmppath;
> + char *newpath;
> + int fdout;
> +
> + fdout = maildir_open_tmp (ctx, dir, &tmppath, &newpath);
> + if (fdout < 0) {
> + return FALSE;
> + }
> +
> + close (fdout);
> + unlink (tmppath);
> + return FALSE;
> +}
> +
> int
> notmuch_insert_command (void *ctx, int argc, char *argv[])
> {
> notmuch_config_t *config;
> notmuch_database_t *notmuch;
> const char *db_path;
> + char *maildir;
> + notmuch_bool_t ret;
>
> config = notmuch_config_open (ctx, NULL, NULL);
> if (config == NULL)
> @@ -33,11 +107,15 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
>
> db_path = notmuch_config_get_database_path (config);
>
> + maildir = talloc_asprintf (ctx, "%s", db_path);
> +
> if (notmuch_database_open (notmuch_config_get_database_path (config),
> NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much))
> return 1;
>
> + ret = insert_message (ctx, notmuch, STDIN_FILENO, maildir);
> +
> notmuch_database_destroy (notmuch);
>
> - return 1;
> + return (ret) ? 0 : 1;
> }
> --
> 1.7.4.4
>
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list