48/fd4cb04bb77df2feb3df9be98dac017e3cfd3b

   1 Return-Path: <jani@nikula.org>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 6FA95431FB6\r
   6         for <notmuch@notmuchmail.org>; Tue, 22 Jan 2013 13:45:50 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id UtWJbfZMCSWn for <notmuch@notmuchmail.org>;\r
  16         Tue, 22 Jan 2013 13:45:49 -0800 (PST)\r
  17 Received: from mail-la0-f54.google.com (mail-la0-f54.google.com\r
  18         [209.85.215.54]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
  19         (No client certificate requested)\r
  20         by olra.theworths.org (Postfix) with ESMTPS id 944B2431FAF\r
  21         for <notmuch@notmuchmail.org>; Tue, 22 Jan 2013 13:45:48 -0800 (PST)\r
  22 Received: by mail-la0-f54.google.com with SMTP id gw10so7085995lab.41\r
  23         for <notmuch@notmuchmail.org>; Tue, 22 Jan 2013 13:45:47 -0800 (PST)\r
  24 X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;\r
  25         d=google.com; s=20120113;\r
  26         h=x-received:from:to:subject:in-reply-to:references:user-agent:date\r
  27         :message-id:mime-version:content-type:content-transfer-encoding\r
  28         :x-gm-message-state;\r
  29         bh=+O/d8PJKF9Oye9+3pmLLre214k7HdRAgWA9ZxFTcfZs=;\r
  30         b=X68Xuy0zoUo8wecnYSBIVZQwL7EwEYAnVxJDQsXKsh8fdC3DwneFcgSJljHsawW5AF\r
  31         aQKRDMJs+t1koQ/AAjbQZOAJu1LAG1UetBgViuqmv4zNi20PJKdE1o+obrtxjiMBY1c5\r
  32         /T2hUe7FY+ZKgOSfYb2/4SQMjWzergDBL0K28vrJERXGCZj0kHxSUq5gMTnWSqZ7ygdp\r
  33         AdHeEvvYQjPhVO4SYdmCb7Vl+WWOwZbzlmSLBFDKKBNlupTmkcGe4aDCv6fDbyLVvqBZ\r
  34         KINQi3+v0lIfG+YCzW6ns9qJqPMkKdlnPZM87J8fbZmImQ4wi77P8d2yCAyAkQbDvFeT\r
  35         Y5sg==\r
  36 X-Received: by 10.112.23.34 with SMTP id j2mr9813037lbf.118.1358891145572;\r
  37         Tue, 22 Jan 2013 13:45:45 -0800 (PST)\r
  38 Received: from localhost (dsl-hkibrasgw4-50df51-27.dhcp.inet.fi.\r
  39         [80.223.81.27])\r
  40         by mx.google.com with ESMTPS id ft8sm7511229lab.9.2013.01.22.13.45.43\r
  41         (version=TLSv1.2 cipher=RC4-SHA bits=128/128);\r
  42         Tue, 22 Jan 2013 13:45:44 -0800 (PST)\r
  43 From: Jani Nikula <jani@nikula.org>\r
  44 To: Peter Wang <novalazy@gmail.com>, notmuch@notmuchmail.org\r
  45 Subject: Re: [PATCH v3 01/20] cli: add stub for insert command\r
  46 In-Reply-To: <1358643004-14522-2-git-send-email-novalazy@gmail.com>\r
  47 References: <1358643004-14522-1-git-send-email-novalazy@gmail.com>\r
  48         <1358643004-14522-2-git-send-email-novalazy@gmail.com>\r
  49 User-Agent: Notmuch/0.14+255~gff3cc55 (http://notmuchmail.org) Emacs/24.2.1\r
  50         (x86_64-pc-linux-gnu)\r
  51 Date: Tue, 22 Jan 2013 23:45:43 +0200\r
  52 Message-ID: <87vcaoj3i0.fsf@nikula.org>\r
  53 MIME-Version: 1.0\r
  54 Content-Type: text/plain; charset=utf-8\r
  55 Content-Transfer-Encoding: quoted-printable\r
  56 X-Gm-Message-State:\r
  57  ALoCoQkJd7fmbnhlqI0o+eYsHjLUH6tK3udqP0ZQgTwJ+I38rbLOueoL52URBy6v2sC4V4vjLHS8\r
  58 X-BeenThere: notmuch@notmuchmail.org\r
  59 X-Mailman-Version: 2.1.13\r
  60 Precedence: list\r
  61 List-Id: "Use and development of the notmuch mail system."\r
  62         <notmuch.notmuchmail.org>\r
  63 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  64         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  65 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  66 List-Post: <mailto:notmuch@notmuchmail.org>\r
  67 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  68 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  69         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  70 X-List-Received-Date: Tue, 22 Jan 2013 21:45:50 -0000\r
  71 \r
  72 \r
  73 Hi Peter -\r
  74 \r
  75 On Sun, 20 Jan 2013, Peter Wang <novalazy@gmail.com> wrote:\r
  76 > The notmuch insert command should read a message from standard input\r
  77 > and deliver it to a Maildir folder, and then incorporate the message\r
  78 > into the notmuch database.  Essentially it moves the functionality of\r
  79 > notmuch-deliver into notmuch.\r
  80 \r
  81 Bikeshedding first, I'd prefer "notmuch deliver" for the command too...\r
  82 \r
  83 > Though it could be used as an alternative to notmuch new, the reason\r
  84 > I want this is to allow my notmuch frontend to add postponed or sent\r
  85 > messages to the mail store and notmuch database, without resorting to\r
  86 > another tool (e.g. notmuch-deliver) nor directly modifying the maildir.\r
  87 \r
  88 This review is based on the following patches squashed together:\r
  89 \r
  90         cli: add stub for insert command\r
  91         insert: open Maildir tmp file\r
  92         insert: copy stdin to Maildir tmp file\r
  93         insert: move file from Maildir tmp to new\r
  94         insert: add new message to database\r
  95         insert: apply default tags to new message\r
  96         insert: parse and apply command-line tag operations\r
  97         insert: fsync after writing tmp file\r
  98         insert: trap SIGINT and clean up\r
  99         insert: add copyright line from notmuch-deliver\r
 100 \r
 101 It's much easier for me to grasp the big picture this way.\r
 102 \r
 103 > ---\r
 104 >  Makefile.local   |    1 +\r
 105 >  notmuch-client.h |    3 +\r
 106 >  notmuch-insert.c |  316 ++++++++++++++++++++++++++++++++++++++++++++++++=\r
 107 ++++++\r
 108 >  notmuch.c        |    3 +\r
 109 >  4 files changed, 323 insertions(+)\r
 110 >  create mode 100644 notmuch-insert.c\r
 111 >\r
 112 > diff --git a/Makefile.local b/Makefile.local\r
 113 > index c274f07..bb2381d 100644\r
 114 > --- a/Makefile.local\r
 115 > +++ b/Makefile.local\r
 116 > @@ -261,6 +261,7 @@ notmuch_client_srcs =3D           \\r
 117 >       notmuch-config.c        \\r
 118 >       notmuch-count.c         \\r
 119 >       notmuch-dump.c          \\r
 120 > +     notmuch-insert.c        \\r
 121 >       notmuch-new.c           \\r
 122 >       notmuch-reply.c         \\r
 123 >       notmuch-restore.c       \\r
 124 > diff --git a/notmuch-client.h b/notmuch-client.h\r
 125 > index 5f28836..af7d094 100644\r
 126 > --- a/notmuch-client.h\r
 127 > +++ b/notmuch-client.h\r
 128 > @@ -175,6 +175,9 @@ int\r
 129 >  notmuch_dump_command (void *ctx, int argc, char *argv[]);\r
 130 >=20=20\r
 131 >  int\r
 132 > +notmuch_insert_command (void *ctx, int argc, char *argv[]);\r
 133 > +\r
 134 > +int\r
 135 >  notmuch_new_command (void *ctx, int argc, char *argv[]);\r
 136 >=20=20\r
 137 >  int\r
 138 > diff --git a/notmuch-insert.c b/notmuch-insert.c\r
 139 > new file mode 100644\r
 140 > index 0000000..0e74be0\r
 141 > --- /dev/null\r
 142 > +++ b/notmuch-insert.c\r
 143 > @@ -0,0 +1,316 @@\r
 144 > +/* notmuch - Not much of an email program, (just index and search)\r
 145 > + *\r
 146 > + * Copyright =C2=A9 2013 Peter Wang\r
 147 > + *\r
 148 > + * Based in part on notmuch-deliver\r
 149 > + * Copyright =C2=A9 2010 Ali Polatel\r
 150 > + *\r
 151 > + * This program is free software: you can redistribute it and/or modify\r
 152 > + * it under the terms of the GNU General Public License as published by\r
 153 > + * the Free Software Foundation, either version 3 of the License, or\r
 154 > + * (at your option) any later version.\r
 155 > + *\r
 156 > + * This program is distributed in the hope that it will be useful,\r
 157 > + * but WITHOUT ANY WARRANTY; without even the implied warranty of\r
 158 > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the\r
 159 > + * GNU General Public License for more details.\r
 160 > + *\r
 161 > + * You should have received a copy of the GNU General Public License\r
 162 > + * along with this program.  If not, see http://www.gnu.org/licenses/ .\r
 163 > + *\r
 164 > + * Author: Peter Wang <novalazy@gmail.com>\r
 165 > + */\r
 166 > +\r
 167 > +#include "notmuch-client.h"\r
 168 > +#include "tag-util.h"\r
 169 > +\r
 170 > +#include <sys/types.h>\r
 171 > +#include <sys/stat.h>\r
 172 > +#include <fcntl.h>\r
 173 > +\r
 174 > +static volatile sig_atomic_t interrupted;\r
 175 > +\r
 176 > +static void\r
 177 > +handle_sigint (unused (int sig))\r
 178 > +{\r
 179 > +    static char msg[] =3D "Stopping...         \n";\r
 180 > +\r
 181 > +    /* This write is "opportunistic", so it's okay to ignore the\r
 182 > +     * result.  It is not required for correctness, and if it does\r
 183 > +     * fail or produce a short write, we want to get out of the signal\r
 184 > +     * handler as quickly as possible, not retry it. */\r
 185 > +    IGNORE_RESULT (write (2, msg, sizeof (msg) - 1));\r
 186 > +    interrupted =3D 1;\r
 187 > +}\r
 188 > +\r
 189 > +/* Like gethostname but guarantees that a null-terminated hostname is\r
 190 > + * returned, even if it has to make one up.\r
 191 > + * Returns true unless hostname contains a slash. */\r
 192 > +static notmuch_bool_t\r
 193 > +safe_gethostname (char *hostname, size_t len)\r
 194 > +{\r
 195 > +    if (gethostname (hostname, len) =3D=3D -1) {\r
 196 > +     strncpy (hostname, "unknown", len);\r
 197 > +    }\r
 198 > +    hostname[len - 1] =3D '\0';\r
 199 > +\r
 200 > +    return (strchr (hostname, '/') =3D=3D NULL);\r
 201 \r
 202 You could just replace all chars you don't accept with something you\r
 203 do. Add ':' to the list of unacceptable chars.\r
 204 \r
 205 > +}\r
 206 > +\r
 207 > +/* Open a unique file in the Maildir 'tmp' directory.\r
 208 > + * Returns the file descriptor on success, or -1 on failure.\r
 209 > + * On success, file paths for the message in the 'tmp' and 'new'\r
 210 > + * directories are returned via tmppath and newpath. */\r
 211 > +static int\r
 212 > +maildir_open_tmp_file (void *ctx, const char *dir,\r
 213 > +                    char **tmppath, char **newpath)\r
 214 > +{\r
 215 > +    pid_t pid;\r
 216 > +    char hostname[256];\r
 217 > +    struct timeval tv;\r
 218 > +    char *filename;\r
 219 > +    int fd =3D -1;\r
 220 > +\r
 221 > +    /* We follow the Dovecot file name generation algorithm. */\r
 222 \r
 223 See also http://cr.yp.to/proto/maildir.html\r
 224 \r
 225 > +    pid =3D getpid ();\r
 226 > +    if (! safe_gethostname (hostname, sizeof (hostname))) {\r
 227 > +     fprintf (stderr, "Error: invalid host name.\n");\r
 228 > +     return -1;\r
 229 > +    }\r
 230 > +    do {\r
 231 > +     gettimeofday (&tv, NULL);\r
 232 > +     filename =3D talloc_asprintf (ctx, "%ld.M%ldP%d.%s",\r
 233 > +                                 tv.tv_sec, tv.tv_usec, pid, hostname);\r
 234 > +     if (! filename) {\r
 235 > +         fprintf (stderr, "Out of memory\n");\r
 236 > +         return -1;\r
 237 > +     }\r
 238 > +\r
 239 > +     *tmppath =3D talloc_asprintf (ctx, "%s/tmp/%s", dir, filename);\r
 240 > +     if (! *tmppath) {\r
 241 > +         fprintf (stderr, "Out of memory\n");\r
 242 > +         return -1;\r
 243 > +     }\r
 244 > +\r
 245 > +     fd =3D open (*tmppath, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0600);\r
 246 > +    } while (fd =3D=3D -1 && errno =3D=3D EEXIST);\r
 247 > +\r
 248 > +    if (fd =3D=3D -1) {\r
 249 > +     fprintf (stderr, "Error: opening %s: %s\n", *tmppath, strerror (errno));\r
 250 > +     return -1;\r
 251 > +    }\r
 252 > +\r
 253 > +    *newpath =3D talloc_asprintf (ctx, "%s/new/%s", dir, filename);\r
 254 > +    if (! *newpath) {\r
 255 > +     fprintf (stderr, "Out of memory\n");\r
 256 > +     close (fd);\r
 257 > +     unlink (*tmppath);\r
 258 > +     return -1;\r
 259 > +    }\r
 260 > +\r
 261 > +    talloc_free (filename);\r
 262 \r
 263 Nitpick, in theory the do-while loop above could allocate a bunch of\r
 264 filenames and paths that you do not free (they're freed as part of the\r
 265 context).\r
 266 \r
 267 > +\r
 268 > +    return fd;\r
 269 > +}\r
 270 > +\r
 271 > +/* Atomically move the new message file from the Maildir 'tmp' directory\r
 272 > + * to the 'new' directory.\r
 273 > + *\r
 274 > + * We follow the Dovecot recommendation to simply use rename()\r
 275 > + * instead of link() and unlink().  See also:\r
 276 > + * http://wiki.dovecot.org/MailboxFormat/Maildir#Mail_delivery\r
 277 > + */\r
 278 > +static notmuch_bool_t\r
 279 > +maildir_move_tmp_to_new (const char *tmppath, const char *newpath)\r
 280 > +{\r
 281 > +    if (rename (tmppath, newpath) !=3D 0) {\r
 282 > +     fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));\r
 283 > +     return FALSE;\r
 284 > +    }\r
 285 > +\r
 286 > +    return TRUE;\r
 287 > +}\r
 288 \r
 289 IMO you could just use rename() inline in the caller, without a wrapper.\r
 290 \r
 291 > +\r
 292 > +/* Copy the contents of standard input (fdin) into fdout. */\r
 293 > +static notmuch_bool_t\r
 294 > +copy_stdin (int fdin, int fdout)\r
 295 \r
 296 The comment and the function name imply the function has something to do\r
 297 with stdin, while it only cares about file descriptors.\r
 298 \r
 299 > +{\r
 300 > +    char buf[4096];\r
 301 > +    char *p;\r
 302 > +    ssize_t remain;\r
 303 > +    ssize_t written;\r
 304 > +\r
 305 > +    while (! interrupted) {\r
 306 > +     remain =3D read (fdin, buf, sizeof (buf));\r
 307 > +     if (remain =3D=3D 0)\r
 308 > +         break;\r
 309 > +     if (remain < 0) {\r
 310 > +         if (errno =3D=3D EINTR)\r
 311 > +             continue;\r
 312 > +         fprintf (stderr, "Error: reading from standard input: %s\n",\r
 313 > +                  strerror (errno));\r
 314 > +         return FALSE;\r
 315 > +     }\r
 316 > +\r
 317 > +     p =3D buf;\r
 318 > +     do {\r
 319 > +         written =3D write (fdout, p, remain);\r
 320 > +         if (written =3D=3D 0)\r
 321 > +             return FALSE;\r
 322 \r
 323 No error message?\r
 324 \r
 325 > +         if (written < 0) {\r
 326 > +             if (errno =3D=3D EINTR)\r
 327 > +                 continue;\r
 328 > +             fprintf (stderr, "Error: writing to temporary file: %s",\r
 329 > +                      strerror (errno));\r
 330 > +             return FALSE;\r
 331 > +         }\r
 332 > +         p +=3D written;\r
 333 > +         remain -=3D written;\r
 334 > +     } while (remain > 0);\r
 335 > +    }\r
 336 > +\r
 337 > +    return ! interrupted;\r
 338 > +}\r
 339 > +\r
 340 > +/* Add the specified message file to the notmuch database, applying tags.\r
 341 > + * The file is renamed to encode notmuch tags as maildir flags. */\r
 342 > +static notmuch_bool_t\r
 343 > +add_file_to_database (notmuch_database_t *notmuch, const char *path,\r
 344 > +                   tag_op_list_t *tag_ops)\r
 345 > +{\r
 346 > +    notmuch_message_t *message;\r
 347 > +    notmuch_status_t status;\r
 348 > +\r
 349 > +    status =3D notmuch_database_add_message (notmuch, path, &message);\r
 350 > +    switch (status) {\r
 351 > +    case NOTMUCH_STATUS_SUCCESS:\r
 352 > +     break;\r
 353 > +    case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:\r
 354 > +     fprintf (stderr, "Warning: duplicate message.\n");\r
 355 \r
 356 This is not uncommon. Why the warning?\r
 357 \r
 358 Also, notmuch new does not apply new.tags in this case. Are you sure we\r
 359 want to do that here? (You get mail, you read and archive it, you get\r
 360 the dupe, it pops up unread in your inbox again.)\r
 361 \r
 362 > +     break;\r
 363 > +    default:\r
 364 > +    case NOTMUCH_STATUS_FILE_NOT_EMAIL:\r
 365 > +    case NOTMUCH_STATUS_READ_ONLY_DATABASE:\r
 366 > +    case NOTMUCH_STATUS_XAPIAN_EXCEPTION:\r
 367 > +    case NOTMUCH_STATUS_OUT_OF_MEMORY:\r
 368 > +    case NOTMUCH_STATUS_FILE_ERROR:\r
 369 > +    case NOTMUCH_STATUS_NULL_POINTER:\r
 370 > +    case NOTMUCH_STATUS_TAG_TOO_LONG:\r
 371 > +    case NOTMUCH_STATUS_UNBALANCED_FREEZE_THAW:\r
 372 > +    case NOTMUCH_STATUS_UNBALANCED_ATOMIC:\r
 373 > +    case NOTMUCH_STATUS_LAST_STATUS:\r
 374 > +     fprintf (stderr, "Error: failed to add `%s' to notmuch database: %s\n",\r
 375 > +              path, notmuch_status_to_string (status));\r
 376 > +     return FALSE;\r
 377 > +    }\r
 378 > +\r
 379 > +    tag_op_list_apply (message, tag_ops, TAG_FLAG_MAILDIR_SYNC);\r
 380 \r
 381 Check return value.\r
 382 \r
 383 > +\r
 384 > +    notmuch_message_destroy (message);\r
 385 > +\r
 386 > +    return TRUE;\r
 387 > +}\r
 388 > +\r
 389 > +static notmuch_bool_t\r
 390 > +insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,\r
 391 > +             const char *dir, tag_op_list_t *tag_ops)\r
 392 > +{\r
 393 > +    char *tmppath;\r
 394 > +    char *newpath;\r
 395 > +    int fdout;\r
 396 > +    notmuch_bool_t ret;\r
 397 > +\r
 398 > +    fdout =3D maildir_open_tmp_file (ctx, dir, &tmppath, &newpath);\r
 399 > +    if (fdout < 0) {\r
 400 > +     return FALSE;\r
 401 > +    }\r
 402 > +    ret =3D copy_stdin (fdin, fdout);\r
 403 > +    if (ret && fsync (fdout) !=3D 0) {\r
 404 \r
 405 Keep ret check and fsync separate.\r
 406 \r
 407 On some file systems you need to fsync the directory after adding a new\r
 408 file as well.\r
 409 \r
 410 > +     fprintf (stderr, "Error: fsync failed: %s\n", strerror (errno));\r
 411 > +     ret =3D FALSE;\r
 412 > +    }\r
 413 > +    close (fdout);\r
 414 > +    if (ret) {\r
 415 > +     ret =3D maildir_move_tmp_to_new (tmppath, newpath);\r
 416 > +    }\r
 417 > +    if (!ret) {\r
 418 > +     unlink (tmppath);\r
 419 > +     return FALSE;\r
 420 > +    }\r
 421 \r
 422 I think you should clean up all of the error paths above. It's not\r
 423 obvious at a glance what happens. Typically it should be just blocks\r
 424 like this repeated:\r
 425 \r
 426         ret =3D foo();\r
 427         if (ret) {\r
 428             /* error handling */\r
 429         }\r
 430 \r
 431 > +\r
 432 > +    ret =3D add_file_to_database (notmuch, newpath, tag_ops);\r
 433 > +    if (!ret) {\r
 434 > +     /* XXX maybe there should be an option to keep the file in maildir? */\r
 435 \r
 436 Yes, in the future.\r
 437 \r
 438 I might like it better if you separated writing the file to maildir and\r
 439 adding the file to the database in the top level\r
 440 notmuch_insert_command() function. The delivery function could return a\r
 441 char * to the filename so it could be passed to\r
 442 add_file_to_database(). And you wouldn't need to pass database or tag\r
 443 ops to the maildir writing function, keeping things clearly separated.\r
 444 \r
 445 > +     unlink (newpath);\r
 446 > +     return FALSE;\r
 447 > +    }\r
 448 > +\r
 449 > +    return TRUE;\r
 450 > +}\r
 451 > +\r
 452 > +int\r
 453 > +notmuch_insert_command (void *ctx, int argc, char *argv[])\r
 454 > +{\r
 455 > +    notmuch_config_t *config;\r
 456 > +    notmuch_database_t *notmuch;\r
 457 > +    struct sigaction action;\r
 458 > +    const char *db_path;\r
 459 > +    const char **new_tags;\r
 460 > +    size_t new_tags_length;\r
 461 > +    tag_op_list_t *tag_ops;\r
 462 > +    char *query_string =3D NULL;\r
 463 > +    char *maildir;\r
 464 > +    int opt_index =3D 1;\r
 465 > +    unsigned int i;\r
 466 > +    notmuch_bool_t ret;\r
 467 > +\r
 468 > +    config =3D notmuch_config_open (ctx, NULL, NULL);\r
 469 > +    if (config =3D=3D NULL)\r
 470 > +     return 1;\r
 471 > +\r
 472 > +    db_path =3D notmuch_config_get_database_path (config);\r
 473 > +    new_tags =3D notmuch_config_get_new_tags (config, &new_tags_length);\r
 474 > +\r
 475 > +    tag_ops =3D tag_op_list_create (ctx);\r
 476 > +    if (tag_ops =3D=3D NULL) {\r
 477 > +     fprintf (stderr, "Out of memory.\n");\r
 478 > +     return 1;\r
 479 > +    }\r
 480 > +    for (i =3D 0; i < new_tags_length; i++) {\r
 481 > +     if (tag_op_list_append (tag_ops, new_tags[i], FALSE))\r
 482 > +         return 1;\r
 483 > +    }\r
 484 > +\r
 485 > +    if (parse_tag_command_line (ctx, argc - opt_index, argv + opt_index,\r
 486 > +                             &query_string, tag_ops))\r
 487 > +     return 1;\r
 488 \r
 489 Never mind about my earlier comment about changing\r
 490 parse_tag_command_line(). It's probably better to allow this to override\r
 491 new.tags.\r
 492 \r
 493 > +\r
 494 > +    if (*query_string !=3D '\0') {\r
 495 > +     fprintf (stderr, "Error: unexpected query string: %s\n", query_string);\r
 496 > +     return 1;\r
 497 > +    }\r
 498 > +\r
 499 > +    maildir =3D talloc_asprintf (ctx, "%s", db_path);\r
 500 > +    if (! maildir) {\r
 501 > +     fprintf (stderr, "Out of memory\n");\r
 502 > +     return 1;\r
 503 > +    }\r
 504 \r
 505 There's no need to talloc maildir; it's the same as db_path.\r
 506 \r
 507 > +\r
 508 > +    /* Setup our handler for SIGINT. We do not set SA_RESTART so that co=\r
 509 pying\r
 510 > +     * from standard input may be interrupted. */\r
 511 > +    memset (&action, 0, sizeof (struct sigaction));\r
 512 > +    action.sa_handler =3D handle_sigint;\r
 513 > +    sigemptyset (&action.sa_mask);\r
 514 > +    action.sa_flags =3D 0;\r
 515 > +    sigaction (SIGINT, &action, NULL);\r
 516 > +\r
 517 > +    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
 518 > +                            NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))\r
 519 > +     return 1;\r
 520 > +\r
 521 > +    ret =3D insert_message (ctx, notmuch, STDIN_FILENO, maildir, tag_ops=\r
 522 );\r
 523 > +\r
 524 > +    notmuch_database_destroy (notmuch);\r
 525 > +\r
 526 > +    return (ret) ? 0 : 1;\r
 527 > +}\r
 528 > diff --git a/notmuch.c b/notmuch.c\r
 529 > index 4fc0973..1c3b893 100644\r
 530 > --- a/notmuch.c\r
 531 > +++ b/notmuch.c\r
 532 > @@ -53,6 +53,9 @@ static command_t commands[] =3D {\r
 533 >      { "new", notmuch_new_command,\r
 534 >        "[options...]",\r
 535 >        "Find and import new messages to the notmuch database." },\r
 536 > +    { "insert", notmuch_insert_command,\r
 537 > +      "[options...] [--] [+<tag>|-<tag> ...] < message",\r
 538 > +      "Add a new message into the maildir and notmuch database." },\r
 539 >      { "search", notmuch_search_command,\r
 540 >        "[options...] <search-terms> [...]",\r
 541 >        "Search for messages matching the given search terms." },\r
 542 > --=20\r
 543 > 1.7.10.4\r