c3/9b63f98171560a298413c03e2bf4c31d8d376f

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