fa/6695e989fdb606612b33b7f97f2a313ccadc81

   1 Return-Path: <bremner@unb.ca>\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 20BDF431FB6\r
   6         for <notmuch@notmuchmail.org>; Fri, 21 Dec 2012 05:30:09 -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\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]\r
  12         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 lMlEewZrehMO for <notmuch@notmuchmail.org>;\r
  16         Fri, 21 Dec 2012 05:30:07 -0800 (PST)\r
  17 Received: from tesseract.cs.unb.ca (tesseract.cs.unb.ca [131.202.240.238])\r
  18         (using TLSv1 with cipher AES256-SHA (256/256 bits))\r
  19         (No client certificate requested)\r
  20         by olra.theworths.org (Postfix) with ESMTPS id 7468F431FAF\r
  21         for <notmuch@notmuchmail.org>; Fri, 21 Dec 2012 05:30:07 -0800 (PST)\r
  22 Received: from fctnnbsc30w-156034082078.dhcp-dynamic.fibreop.nb.bellaliant.net\r
  23         ([156.34.82.78] helo=zancas.localnet)\r
  24         by tesseract.cs.unb.ca with esmtpsa\r
  25         (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.72)\r
  26         (envelope-from <bremner@unb.ca>)\r
  27         id 1Tm2fq-0005k8-Kn; Fri, 21 Dec 2012 09:30:06 -0400\r
  28 Received: from bremner by zancas.localnet with local (Exim 4.80)\r
  29         (envelope-from <bremner@unb.ca>)\r
  30         id 1Tm2fl-000755-4j; Fri, 21 Dec 2012 09:29:57 -0400\r
  31 From: David Bremner <david@tethera.net>\r
  32 To: notmuch@notmuchmail.org\r
  33 Subject: Re: [Patch v8 01/18] parse_tag_line: use enum for return value.\r
  34 In-Reply-To: <1356095307-22895-1-git-send-email-david@tethera.net>\r
  35 References: <1356095307-22895-1-git-send-email-david@tethera.net>\r
  36 User-Agent: Notmuch/0.14+163~g11a220a (http://notmuchmail.org) Emacs/24.2.1\r
  37         (x86_64-pc-linux-gnu)\r
  38 Date: Fri, 21 Dec 2012 09:29:57 -0400\r
  39 Message-ID: <87sj6z7ctm.fsf@zancas.localnet>\r
  40 MIME-Version: 1.0\r
  41 Content-Type: text/plain\r
  42 X-Spam_bar: -\r
  43 X-BeenThere: notmuch@notmuchmail.org\r
  44 X-Mailman-Version: 2.1.13\r
  45 Precedence: list\r
  46 List-Id: "Use and development of the notmuch mail system."\r
  47         <notmuch.notmuchmail.org>\r
  48 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  49         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  50 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  51 List-Post: <mailto:notmuch@notmuchmail.org>\r
  52 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  53 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  54         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  55 X-List-Received-Date: Fri, 21 Dec 2012 13:30:09 -0000\r
  56 \r
  57 david@tethera.net writes:\r
  58 \r
  59 > From: David Bremner <bremner@debian.org>\r
  60 >\r
  61 > This is essentially cosmetic, since success=0 is promised by\r
  62 > the comments in tag-utils.h.\r
  63 \r
  64 In an amazing display of skill and style, in attempting to abort a git\r
  65 send-email run so that I could rebase that last fixup away, I managed to\r
  66 send the series without the cover letter. So, no point sending the whole\r
  67 thing again just for that.\r
  68 \r
  69 This ever-growing series obsoletes \r
  70 \r
  71      id:1355492062-7546-1-git-send-email-david@tethera.net\r
  72 \r
  73 I think I decided to ignore \r
  74 \r
  75       id:871uettxln.fsf@qmul.ac.uk\r
  76 \r
  77 Perhaps that should be documented, I'm not sure. Since the batch tagging\r
  78 interface also allows you to remove strangely named tags, I think it is\r
  79 ok.\r
  80 \r
  81 Hopefully the new man page clears up what is and isn't allowed for\r
  82 unencoded characters. Since spaces are the only thing used as\r
  83 (top-level) delimiters now, they are the only thing "compressed" by\r
  84 strtok_len.\r
  85 \r
  86 For id:87vcc2q5n2.fsf@nikula.org, there is a seperate series\r
  87 id:1355716788-2940-1-git-send-email-david@tethera.net to fix the memory\r
  88 leak/allocation confusion. This series needs to be rebased onto the\r
  89 final version of that one. I decided _not_ to relax the requirement that\r
  90 the ':' marking a prefix be un-encoded, but rather update the\r
  91 documentation.\r
  92 \r
  93 A summary of changes to the series follows; I left out the interdiff for\r
  94 the notmuch-tag man page, and things that got their own new commit.\r
  95 \r
  96 ----------------------------------------------------------------------\r
  97 commit 30adcab7678296b22d86da06d472c3920c336747\r
  98 Author: David Bremner <bremner@debian.org>\r
  99 Date:   Sat Dec 15 15:17:40 2012 -0400\r
 100 \r
 101     fixup: clarify TAG_FLAG_ID_ONLY comments and name\r
 102 \r
 103 diff --git a/notmuch-restore.c b/notmuch-restore.c\r
 104 index 112f2f3..1b66e76 100644\r
 105 --- a/notmuch-restore.c\r
 106 +++ b/notmuch-restore.c\r
 107 @@ -208,7 +208,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])\r
 108         if (input_format == DUMP_FORMAT_SUP) {\r
 109             ret = parse_sup_line (ctx, line, &query_string, tag_ops);\r
 110         } else {\r
 111 -           ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS | TAG_FLAG_ID_ONLY,\r
 112 +           ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS | TAG_FLAG_ID_DIRECT,\r
 113                                   &query_string, tag_ops);\r
 114         }\r
 115  \r
 116 diff --git a/tag-util.c b/tag-util.c\r
 117 index 8fea76c..37bffd5 100644\r
 118 --- a/tag-util.c\r
 119 +++ b/tag-util.c\r
 120 @@ -201,7 +201,7 @@ parse_tag_line (void *ctx, char *line,\r
 121      }\r
 122  \r
 123      /* tok now points to the query string */\r
 124 -    if (flags & TAG_FLAG_ID_ONLY) {\r
 125 +    if (flags & TAG_FLAG_ID_DIRECT) {\r
 126         /* this is under the assumption that any whitespace in the\r
 127          * message-id must be hex-encoded. The check is probably not\r
 128          * perfect for exotic unicode whitespace; as fallback the\r
 129 diff --git a/tag-util.h b/tag-util.h\r
 130 index 7674051..eec00cf 100644\r
 131 --- a/tag-util.h\r
 132 +++ b/tag-util.h\r
 133 @@ -28,8 +28,10 @@ typedef enum {\r
 134       */\r
 135      TAG_FLAG_BE_GENEROUS = (1 << 3),\r
 136  \r
 137 -    /* Query consists of a single id:$message-id */\r
 138 -    TAG_FLAG_ID_ONLY = (1 << 4)\r
 139 +    /* Directly look up messages by hex-decoded message-id, rather\r
 140 +     * than parsing a general query. The query MUST be of the form\r
 141 +     * id:$message-id. */\r
 142 +    TAG_FLAG_ID_DIRECT = (1 << 4)\r
 143  \r
 144  } tag_op_flag_t;\r
 145  \r
 146 \r
 147 commit 256ec05a347b949e6b2bda0d2b6902ed8ab3fab3\r
 148 Author: David Bremner <bremner@debian.org>\r
 149 Date:   Sat Dec 15 19:54:05 2012 -0400\r
 150 \r
 151     fixup for id:87sj778ajb.fsf@qmul.ac.uk and id:87zk1fot39.fsf@nikula.org\r
 152 \r
 153 diff --git a/notmuch-tag.c b/notmuch-tag.c\r
 154 index 44b5bb4..5c8bad4 100644\r
 155 --- a/notmuch-tag.c\r
 156 +++ b/notmuch-tag.c\r
 157 @@ -65,14 +65,16 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,\r
 158      else\r
 159         query_string = talloc_asprintf (ctx, "( %s ) and (", orig_query_string);\r
 160  \r
 161 -\r
 162      /* Boolean terms surrounded by double quotes can contain any\r
 163       * character.  Double quotes are quoted by doubling them. */\r
 164  \r
 165      for (i = 0; i < tag_op_list_size (list) && query_string; i++) {\r
 166 -       double_quote_str (ctx,\r
 167 -                         tag_op_list_tag (list, i),\r
 168 -                         &escaped, &escaped_len);\r
 169 +       /* XXX in case of OOM, query_string will be deallocated when\r
 170 +        * ctx is, which might be at shutdown */\r
 171 +       if (double_quote_str (ctx,\r
 172 +                             tag_op_list_tag (list, i),\r
 173 +                             &escaped, &escaped_len))\r
 174 +           return NULL;\r
 175  \r
 176         query_string = talloc_asprintf_append_buffer (\r
 177             query_string, "%s%stag:%s", join,\r
 178 diff --git a/util/string-util.c b/util/string-util.c\r
 179 index ea7c25b..b9039f4 100644\r
 180 --- a/util/string-util.c\r
 181 +++ b/util/string-util.c\r
 182 @@ -46,8 +46,11 @@ double_quote_str (void *ctx, const char *str,\r
 183      for (in = str; *in; in++)\r
 184         needed += (*in == '"') ? 2 : 1;\r
 185  \r
 186 -    if (needed > *len)\r
 187 -       *buf = talloc_realloc (ctx, *buf, char, 2*needed);\r
 188 +    if ((*buf == NULL) || (needed > *len)) {\r
 189 +       *len = 2 * needed;\r
 190 +       *buf = talloc_realloc (ctx, *buf, char, *len);\r
 191 +    }\r
 192 +\r
 193  \r
 194      if (! *buf)\r
 195         return 1;\r
 196 @@ -62,7 +65,7 @@ double_quote_str (void *ctx, const char *str,\r
 197         *out++ = *in++;\r
 198      }\r
 199      *out++ = '"';\r
 200 -    *out = 0;\r
 201 +    *out = '\0';\r
 202  \r
 203      return 0;\r
 204  }\r
 205 diff --git a/util/string-util.h b/util/string-util.h\r
 206 index b593bc7..4fc7942 100644\r
 207 --- a/util/string-util.h\r
 208 +++ b/util/string-util.h\r
 209 @@ -23,7 +23,7 @@ char *strtok_len (char *s, const char *delim, size_t *len);\r
 210   * Any internal double-quotes are doubled, i.e. a"b -> "a""b"\r
 211   *\r
 212   * Output is into buf; it may be talloc_realloced\r
 213 - * return 0 on success, non-zero on failure.\r
 214 + * Return: 0 on success, non-zero on memory allocation failure.\r
 215   */\r
 216  int double_quote_str (void *talloc_ctx, const char *str,\r
 217                       char **buf, size_t *len);\r
 218 \r
 219 commit 4b26ac6f99c74cced64cfae317e6ac4b6a8f706f\r
 220 Author: David Bremner <bremner@debian.org>\r
 221 Date:   Mon Dec 17 23:36:30 2012 -0400\r
 222 \r
 223     fixup: rewrite decode+quote function for queries.\r
 224     \r
 225     Rather than splitting on ':' at the top level, we examine each space\r
 226     delimited token for a prefix.\r
 227 \r
 228 diff --git a/tag-util.c b/tag-util.c\r
 229 index 37bffd5..b0a846b 100644\r
 230 --- a/tag-util.c\r
 231 +++ b/tag-util.c\r
 232 @@ -56,9 +56,14 @@ illegal_tag (const char *tag, notmuch_bool_t remove)\r
 233      return NULL;\r
 234  }\r
 235  \r
 236 +/* Input is a hex encoded string, presumed to be a query for Xapian.\r
 237 + *\r
 238 + * Space delimited tokens are decoded and quoted, with '*' and prefixes\r
 239 + * of the form "foo:" passed through unquoted.\r
 240 + */\r
 241  static tag_parse_status_t\r
 242 -quote_and_decode_query (void *ctx, char *encoded, const char *line_for_error,\r
 243 -                       char **query_string)\r
 244 +unhex_and_quote (void *ctx, char *encoded, const char *line_for_error,\r
 245 +                char **query_string)\r
 246  {\r
 247      char *tok = encoded;\r
 248      size_t tok_len = 0;\r
 249 @@ -68,14 +73,43 @@ quote_and_decode_query (void *ctx, char *encoded, const char *line_for_error,\r
 250  \r
 251      *query_string = talloc_strdup (ctx, "");\r
 252  \r
 253 -    while (*query_string &&\r
 254 -          (tok = strtok_len (tok + tok_len, ": ", &tok_len)) != NULL) {\r
 255 -       char delim = tok[tok_len];\r
 256 +    while ((tok = strtok_len (tok + tok_len, " ", &tok_len)) != NULL) {\r
 257 +\r
 258 +       size_t prefix_len;\r
 259 +       char delim = *(tok + tok_len);\r
 260  \r
 261         *(tok + tok_len++) = '\0';\r
 262  \r
 263 -       if (strcspn (tok, "%") < tok_len - 1) {\r
 264 -           /* something to decode */\r
 265 +       prefix_len = hex_invariant (tok, tok_len);\r
 266 +\r
 267 +       if ((strcmp (tok, "*") == 0) || prefix_len >= tok_len - 1) {\r
 268 +\r
 269 +           /* pass some things through without quoting or decoding.\r
 270 +            * Note for '*' this is mandatory.\r
 271 +            */\r
 272 +\r
 273 +           if (! (*query_string = talloc_asprintf_append_buffer (\r
 274 +                      *query_string, "%s%c", tok, delim))) {\r
 275 +\r
 276 +               ret = line_error (TAG_PARSE_OUT_OF_MEMORY,\r
 277 +                                 line_for_error, "aborting");\r
 278 +               goto DONE;\r
 279 +           }\r
 280 +\r
 281 +       } else {\r
 282 +           /* potential prefix: one for ':', then something after */\r
 283 +           if ((tok_len - prefix_len > 2) && *(tok + prefix_len) == ':') {\r
 284 +               if (! (*query_string = talloc_strndup_append (*query_string,\r
 285 +                                                             tok,\r
 286 +                                                             prefix_len + 1))) {\r
 287 +                   ret = line_error (TAG_PARSE_OUT_OF_MEMORY,\r
 288 +                                     line_for_error, "aborting");\r
 289 +                   goto DONE;\r
 290 +               }\r
 291 +               tok += prefix_len + 1;\r
 292 +               tok_len -= prefix_len + 1;\r
 293 +           }\r
 294 +\r
 295             if (hex_decode_inplace (tok) != HEX_SUCCESS) {\r
 296                 ret = line_error (TAG_PARSE_INVALID, line_for_error,\r
 297                                   "hex decoding of token '%s' failed", tok);\r
 298 @@ -87,17 +121,14 @@ quote_and_decode_query (void *ctx, char *encoded, const char *line_for_error,\r
 299                                   line_for_error, "aborting");\r
 300                 goto DONE;\r
 301             }\r
 302 -           *query_string = talloc_asprintf_append_buffer (\r
 303 -               *query_string, "%s%c", buf, delim);\r
 304  \r
 305 -       } else {\r
 306 -           /* This is not just an optimization, but used to preserve\r
 307 -            * prefixes like id:, which cannot be quoted.\r
 308 -            */\r
 309 -           *query_string = talloc_asprintf_append_buffer (\r
 310 -               *query_string, "%s%c", tok, delim);\r
 311 +           if (! (*query_string = talloc_asprintf_append_buffer (\r
 312 +                      *query_string, "%s%c", buf, delim))) {\r
 313 +               ret = line_error (TAG_PARSE_OUT_OF_MEMORY,\r
 314 +                                 line_for_error, "aborting");\r
 315 +               goto DONE;\r
 316 +           }\r
 317         }\r
 318 -\r
 319      }\r
 320  \r
 321    DONE:\r
 322 @@ -220,7 +251,7 @@ parse_tag_line (void *ctx, char *line,\r
 323         /* skip 'id:' */\r
 324         *query_string = tok + 3;\r
 325      } else {\r
 326 -       ret = quote_and_decode_query (ctx, tok, line_for_error, query_string);\r
 327 +       ret = unhex_and_quote (ctx, tok, line_for_error, query_string);\r
 328      }\r
 329  \r
 330    DONE:\r
 331 \r
 332 \r
 333 \r