8b/9a9ebba7d479128cbd1b1d341a5fe377a2d652

   1 Return-Path: <m.walters@qmul.ac.uk>\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 7C826431FB6\r
   6         for <notmuch@notmuchmail.org>; Sat, 29 Dec 2012 08:51:02 -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: -1.098\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5\r
  12         tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001,\r
  13         NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled\r
  14 Received: from olra.theworths.org ([127.0.0.1])\r
  15         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  16         with ESMTP id kZ6ZGevh3uL8 for <notmuch@notmuchmail.org>;\r
  17         Sat, 29 Dec 2012 08:50:58 -0800 (PST)\r
  18 Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6])\r
  19         (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
  20         (No client certificate requested)\r
  21         by olra.theworths.org (Postfix) with ESMTPS id 2A211431FAF\r
  22         for <notmuch@notmuchmail.org>; Sat, 29 Dec 2012 08:50:58 -0800 (PST)\r
  23 Received: from smtp.qmul.ac.uk ([138.37.6.40])\r
  24         by mail2.qmul.ac.uk with esmtp (Exim 4.71)\r
  25         (envelope-from <m.walters@qmul.ac.uk>)\r
  26         id 1TozcY-0000bk-68; Sat, 29 Dec 2012 16:50:52 +0000\r
  27 Received: from 94.197.9.104.threembb.co.uk ([94.197.9.104] helo=localhost)\r
  28         by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.69)\r
  29         (envelope-from <m.walters@qmul.ac.uk>)\r
  30         id 1TozcW-0000mF-TG; Sat, 29 Dec 2012 16:50:49 +0000\r
  31 From: Mark Walters <markwalters1009@gmail.com>\r
  32 To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org\r
  33 Subject: Re: [PATCH v3 2/5] util: Function to parse boolean term queries\r
  34 In-Reply-To: <1356719189-2837-3-git-send-email-amdragon@mit.edu>\r
  35 References: <1356719189-2837-1-git-send-email-amdragon@mit.edu>\r
  36         <1356719189-2837-3-git-send-email-amdragon@mit.edu>\r
  37 User-Agent: Notmuch/0.14+236~g1d0044f (http://notmuchmail.org) Emacs/23.4.1\r
  38         (x86_64-pc-linux-gnu)\r
  39 Date: Sat, 29 Dec 2012 16:50:51 +0000\r
  40 Message-ID: <87y5ggssyc.fsf@qmul.ac.uk>\r
  41 MIME-Version: 1.0\r
  42 Content-Type: text/plain; charset=us-ascii\r
  43 X-Sender-Host-Address: 94.197.9.104\r
  44 X-QM-SPAM-Info: Sender has good ham record.  :)\r
  45 X-QM-Body-MD5: df4242471a0c8f99ee5abc9849039f5f (of first 20000 bytes)\r
  46 X-SpamAssassin-Score: -2.3\r
  47 X-SpamAssassin-SpamBar: --\r
  48 X-SpamAssassin-Report: The QM spam filters have analysed this message to\r
  49         determine if it is\r
  50         spam. We require at least 5.0 points to mark a message as spam.\r
  51         This message scored -2.3 points.\r
  52         Summary of the scoring: \r
  53         * -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/,\r
  54         *      medium trust\r
  55         *      [138.37.6.40 listed in list.dnswl.org]\r
  56         * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail\r
  57         provider *      (markwalters1009[at]gmail.com)\r
  58         * -0.0 T_RP_MATCHES_RCVD Envelope sender domain matches handover relay\r
  59         *      domain\r
  60 X-QM-Scan-Virus: ClamAV says the message is clean\r
  61 X-BeenThere: notmuch@notmuchmail.org\r
  62 X-Mailman-Version: 2.1.13\r
  63 Precedence: list\r
  64 List-Id: "Use and development of the notmuch mail system."\r
  65         <notmuch.notmuchmail.org>\r
  66 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  67         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  68 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  69 List-Post: <mailto:notmuch@notmuchmail.org>\r
  70 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  71 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  72         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  73 X-List-Received-Date: Sat, 29 Dec 2012 16:51:02 -0000\r
  74 \r
  75 \r
  76 On Fri, 28 Dec 2012, Austin Clements <amdragon@MIT.EDU> wrote:\r
  77 > This parses the subset of Xapian's boolean term quoting rules that are\r
  78 > used by make_boolean_term.  This is provided as a generic string\r
  79 > utility, but will be used shortly in notmuch restore to parse and\r
  80 > optimize for ID queries.\r
  81 > ---\r
  82 >  util/string-util.c |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++\r
  83 >  util/string-util.h |   11 +++++++++++\r
  84 >  2 files changed, 66 insertions(+)\r
  85 >\r
  86 > diff --git a/util/string-util.c b/util/string-util.c\r
  87 > index e4bea21..83b4953 100644\r
  88 > --- a/util/string-util.c\r
  89 > +++ b/util/string-util.c\r
  90 > @@ -96,3 +96,58 @@ make_boolean_term (void *ctx, const char *prefix, const char *term,\r
  91 >  \r
  92 >      return 0;\r
  93 >  }\r
  94 > +\r
  95 > +int\r
  96 > +parse_boolean_term (void *ctx, const char *str,\r
  97 > +                 char **prefix_out, char **term_out)\r
  98 > +{\r
  99 > +    *prefix_out = *term_out = NULL;\r
 100 > +\r
 101 > +    /* Parse prefix */\r
 102 > +    const char *pos = strchr (str, ':');\r
 103 > +    if (! pos)\r
 104 > +     goto FAIL;\r
 105 > +    *prefix_out = talloc_strndup (ctx, str, pos - str);\r
 106 > +    ++pos;\r
 107 > +\r
 108 > +    /* Implement de-quoting compatible with make_boolean_term. */\r
 109 > +    if (*pos == '"') {\r
 110 > +     char *out = talloc_array (ctx, char, strlen (pos));\r
 111 > +     int closed = 0;\r
 112 > +     *term_out = out;\r
 113 > +     /* Skip the opening quote, find the closing quote, and\r
 114 > +      * un-double doubled internal quotes. */\r
 115 > +     for (++pos; *pos; ) {\r
 116 > +         if (*pos == '"') {\r
 117 > +             ++pos;\r
 118 > +             if (*pos != '"') {\r
 119 > +                 /* Found the closing quote. */\r
 120 > +                 closed = 1;\r
 121 > +                 break;\r
 122 > +             }\r
 123 > +         }\r
 124 > +         *out++ = *pos++;\r
 125 > +     }\r
 126 > +     /* Did the term terminate without a closing quote or is there\r
 127 > +      * trailing text after the closing quote? */\r
 128 > +     if (!closed || *pos)\r
 129 > +         goto FAIL;\r
 130 > +     *out = '\0';\r
 131 > +    } else {\r
 132 > +     const char *start = pos;\r
 133 > +     /* Check for text after the boolean term. */\r
 134 > +     while (*pos > ' ' && *pos != ')')\r
 135 > +         ++pos;\r
 136 > +     if (*pos)\r
 137 > +         goto FAIL;\r
 138 > +     /* No trailing text; dup the string so the caller can free\r
 139 > +      * it. */\r
 140 > +     *term_out = talloc_strdup (ctx, start);\r
 141 > +    }\r
 142 > +    return 0;\r
 143 \r
 144 This looks like it will fail if a line contains trailing white space\r
 145 (and that seemed to be the case from my experiment). Is that wanted?\r
 146 \r
 147 If yes we may want to make the error message clearer as I got an error\r
 148 of the form\r
 149 Warning: cannot parse query: id:message-id@example.com   \r
 150 and the fact that line contained trailing spaces was not clear.\r
 151 \r
 152 > +\r
 153 > + FAIL:\r
 154 > +    talloc_free (*prefix_out);\r
 155 > +    talloc_free (*term_out);\r
 156 > +    return 1;\r
 157 > +}\r
 158 > diff --git a/util/string-util.h b/util/string-util.h\r
 159 > index b8844a3..43d49d0 100644\r
 160 > --- a/util/string-util.h\r
 161 > +++ b/util/string-util.h\r
 162 > @@ -33,4 +33,15 @@ char *strtok_len (char *s, const char *delim, size_t *len);\r
 163 >  int make_boolean_term (void *talloc_ctx, const char *prefix, const char *term,\r
 164 >                      char **buf, size_t *len);\r
 165 >  \r
 166 > +/* Parse a boolean term query produced by make_boolean_term, returning\r
 167 > + * the prefix in *prefix_out and the term in *term_out.  *prefix_out\r
 168 > + * and *term_out will be talloc'd with context ctx.\r
 169 > + *\r
 170 > + * Return: 0 on success, non-zero on parse error (including trailing\r
 171 > + * data in str).\r
 172 > + */\r
 173 \r
 174 I think it would be worth detailing what requirements a boolean term\r
 175 here should meet to be parsed.  In particular, since make_boolean_term\r
 176 is more restrictive than Xapian we should say whether this is like\r
 177 make_boolean_term, like Xapian or somewhere in between.\r
 178 \r
 179 \r
 180 Best wishes\r
 181 \r
 182 Mark\r
 183 \r
 184 \r
 185 > +int\r
 186 > +parse_boolean_term (void *ctx, const char *str,\r
 187 > +                 char **prefix_out, char **term_out);\r
 188 > +\r
 189 >  #endif\r
 190 > -- \r
 191 > 1.7.10.4\r