From: Kevin Boulain <kevin@boula.in>
To: notmuch@notmuchmail.org
Cc: Kevin Boulain <kevin@boula.in>
Subject: [PATCH v2 2/2] lib: thread-safe s-expression query parser
Date: Sun, 27 Aug 2023 14:31:03 +0200 [thread overview]
Message-ID: <20230827123103.22744-2-kevin@boula.in> (raw)
In-Reply-To: <87fs5fg40h.fsf@tethera.net>
Follow-up of 6273966d, now that sfsexp 1.4.1 doesn't rely on globals
anymore by default (https://github.com/mjsottile/sfsexp/issues/21).
This simply defers the initial query generation to use the thread-safe
helper (xapian_query_match_all) instead of Xapian::Query::MatchAll.
---
lib/parse-sexp.cc | 89 +++++++++++++++++++++++++++++------------------
test/T810-tsan.sh | 1 -
2 files changed, 55 insertions(+), 35 deletions(-)
diff --git a/lib/parse-sexp.cc b/lib/parse-sexp.cc
index 9cadbc13..930888e9 100644
--- a/lib/parse-sexp.cc
+++ b/lib/parse-sexp.cc
@@ -3,6 +3,7 @@
#if HAVE_SFSEXP
#include "sexp.h"
#include "unicode-util.h"
+#include "xapian-extra.h"
/* _sexp is used for file scope symbols to avoid clashing with
* definitions from sexp.h */
@@ -39,82 +40,99 @@ typedef enum {
/*
* define bitwise operators to hide casts */
-inline _sexp_flag_t
+static inline _sexp_flag_t
operator| (_sexp_flag_t a, _sexp_flag_t b)
{
return static_cast<_sexp_flag_t>(
static_cast<unsigned>(a) | static_cast<unsigned>(b));
}
-inline _sexp_flag_t
+static inline _sexp_flag_t
operator& (_sexp_flag_t a, _sexp_flag_t b)
{
return static_cast<_sexp_flag_t>(
static_cast<unsigned>(a) & static_cast<unsigned>(b));
}
+typedef enum {
+ SEXP_INITIAL_MATCH_ALL,
+ SEXP_INITIAL_MATCH_NOTHING,
+} _sexp_initial_t;
+
+static inline Xapian::Query
+_sexp_initial_query (_sexp_initial_t initial)
+{
+ switch (initial) {
+ case SEXP_INITIAL_MATCH_ALL:
+ return xapian_query_match_all ();
+ case SEXP_INITIAL_MATCH_NOTHING:
+ return Xapian::Query::MatchNothing;
+ }
+ INTERNAL_ERROR ("invalid initial sexp value %d", initial);
+}
+
typedef struct {
const char *name;
Xapian::Query::op xapian_op;
- Xapian::Query initial;
+ _sexp_initial_t initial;
_sexp_flag_t flags;
} _sexp_prefix_t;
static _sexp_prefix_t prefixes[] =
{
- { "and", Xapian::Query::OP_AND, Xapian::Query::MatchAll,
+ { "and", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_NONE },
- { "attachment", Xapian::Query::OP_AND, Xapian::Query::MatchAll,
+ { "attachment", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD | SEXP_FLAG_WILDCARD | SEXP_FLAG_EXPAND },
- { "body", Xapian::Query::OP_AND, Xapian::Query::MatchAll,
+ { "body", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD },
- { "date", Xapian::Query::OP_INVALID, Xapian::Query::MatchAll,
+ { "date", Xapian::Query::OP_INVALID, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_RANGE },
- { "from", Xapian::Query::OP_AND, Xapian::Query::MatchAll,
+ { "from", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND },
- { "folder", Xapian::Query::OP_OR, Xapian::Query::MatchNothing,
+ { "folder", Xapian::Query::OP_OR, SEXP_INITIAL_MATCH_NOTHING,
SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND |
SEXP_FLAG_PATHNAME },
- { "id", Xapian::Query::OP_OR, Xapian::Query::MatchNothing,
+ { "id", Xapian::Query::OP_OR, SEXP_INITIAL_MATCH_NOTHING,
SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX },
- { "infix", Xapian::Query::OP_INVALID, Xapian::Query::MatchAll,
+ { "infix", Xapian::Query::OP_INVALID, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_SINGLE | SEXP_FLAG_ORPHAN },
- { "is", Xapian::Query::OP_AND, Xapian::Query::MatchAll,
+ { "is", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND },
- { "lastmod", Xapian::Query::OP_INVALID, Xapian::Query::MatchAll,
+ { "lastmod", Xapian::Query::OP_INVALID, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_RANGE },
- { "matching", Xapian::Query::OP_AND, Xapian::Query::MatchAll,
+ { "matching", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_DO_EXPAND },
- { "mid", Xapian::Query::OP_OR, Xapian::Query::MatchNothing,
+ { "mid", Xapian::Query::OP_OR, SEXP_INITIAL_MATCH_NOTHING,
SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX },
- { "mimetype", Xapian::Query::OP_AND, Xapian::Query::MatchAll,
+ { "mimetype", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD | SEXP_FLAG_WILDCARD | SEXP_FLAG_EXPAND },
- { "not", Xapian::Query::OP_AND_NOT, Xapian::Query::MatchAll,
+ { "not", Xapian::Query::OP_AND_NOT, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_NONE },
- { "of", Xapian::Query::OP_AND, Xapian::Query::MatchAll,
+ { "of", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_DO_EXPAND },
- { "or", Xapian::Query::OP_OR, Xapian::Query::MatchNothing,
+ { "or", Xapian::Query::OP_OR, SEXP_INITIAL_MATCH_NOTHING,
SEXP_FLAG_NONE },
- { "path", Xapian::Query::OP_OR, Xapian::Query::MatchNothing,
+ { "path", Xapian::Query::OP_OR, SEXP_INITIAL_MATCH_NOTHING,
SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX |
SEXP_FLAG_PATHNAME },
- { "property", Xapian::Query::OP_AND, Xapian::Query::MatchAll,
+ { "property", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND },
- { "query", Xapian::Query::OP_INVALID, Xapian::Query::MatchNothing,
+ { "query", Xapian::Query::OP_INVALID, SEXP_INITIAL_MATCH_NOTHING,
SEXP_FLAG_SINGLE | SEXP_FLAG_ORPHAN },
- { "regex", Xapian::Query::OP_INVALID, Xapian::Query::MatchAll,
+ { "regex", Xapian::Query::OP_INVALID, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_SINGLE | SEXP_FLAG_DO_REGEX },
- { "rx", Xapian::Query::OP_INVALID, Xapian::Query::MatchAll,
+ { "rx", Xapian::Query::OP_INVALID, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_SINGLE | SEXP_FLAG_DO_REGEX },
- { "starts-with", Xapian::Query::OP_WILDCARD, Xapian::Query::MatchAll,
+ { "starts-with", Xapian::Query::OP_WILDCARD, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_SINGLE },
- { "subject", Xapian::Query::OP_AND, Xapian::Query::MatchAll,
+ { "subject", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND },
- { "tag", Xapian::Query::OP_AND, Xapian::Query::MatchAll,
+ { "tag", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND },
- { "thread", Xapian::Query::OP_OR, Xapian::Query::MatchNothing,
+ { "thread", Xapian::Query::OP_OR, SEXP_INITIAL_MATCH_NOTHING,
SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND },
- { "to", Xapian::Query::OP_AND, Xapian::Query::MatchAll,
+ { "to", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD | SEXP_FLAG_WILDCARD | SEXP_FLAG_EXPAND },
{ }
};
@@ -318,7 +336,8 @@ _sexp_expand_query (notmuch_database_t *notmuch,
return NOTMUCH_STATUS_BAD_QUERY_SYNTAX;
}
- status = _sexp_combine_query (notmuch, NULL, NULL, prefix->xapian_op, prefix->initial, sx,
+ status = _sexp_combine_query (notmuch, NULL, NULL, prefix->xapian_op,
+ _sexp_initial_query (prefix->initial), sx,
subquery);
if (status)
return status;
@@ -370,7 +389,8 @@ _sexp_parse_header (notmuch_database_t *notmuch, const _sexp_prefix_t *parent,
parent = &user_prefix;
- return _sexp_combine_query (notmuch, parent, env, Xapian::Query::OP_AND, Xapian::Query::MatchAll,
+ return _sexp_combine_query (notmuch, parent, env, Xapian::Query::OP_AND,
+ xapian_query_match_all (),
sx->list->next, output);
}
@@ -520,7 +540,7 @@ _sexp_parse_range (notmuch_database_t *notmuch, const _sexp_prefix_t *prefix,
/* empty range matches everything */
if (! sx) {
- output = Xapian::Query::MatchAll;
+ output = xapian_query_match_all ();
return NOTMUCH_STATUS_SUCCESS;
}
@@ -628,7 +648,7 @@ _sexp_to_xapian_query (notmuch_database_t *notmuch, const _sexp_prefix_t *parent
/* Empty list */
if (! sx->list) {
- output = Xapian::Query::MatchAll;
+ output = xapian_query_match_all ();
return NOTMUCH_STATUS_SUCCESS;
}
@@ -704,7 +724,8 @@ _sexp_to_xapian_query (notmuch_database_t *notmuch, const _sexp_prefix_t *parent
return _sexp_expand_query (notmuch, prefix, parent, env, sx->list->next, output);
}
- return _sexp_combine_query (notmuch, parent, env, prefix->xapian_op, prefix->initial,
+ return _sexp_combine_query (notmuch, parent, env, prefix->xapian_op,
+ _sexp_initial_query (prefix->initial),
sx->list->next, output);
}
}
diff --git a/test/T810-tsan.sh b/test/T810-tsan.sh
index de98c0a9..d4482998 100755
--- a/test/T810-tsan.sh
+++ b/test/T810-tsan.sh
@@ -92,7 +92,6 @@ test_expect_equal_file EXPECTED OUTPUT
if [ $NOTMUCH_HAVE_SFSEXP -eq 1 ]; then
test_begin_subtest "sexp query"
- test_subtest_known_broken
test_C ${MAIL_DIR} ${MAIL_DIR}-2 <<EOF
#include <notmuch-test.h>
#include <pthread.h>
next prev parent reply other threads:[~2023-08-27 12:37 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-03 20:31 [PATCH 1/2] test: showcase thread-unsafe s-expression query parser Kevin Boulain
2023-04-03 20:31 ` [PATCH 2/2] lib: thread-safe " Kevin Boulain
2023-07-22 19:06 ` David Bremner
2023-08-27 12:31 ` [PATCH v2 1/2] test: showcase thread-unsafe " Kevin Boulain
2023-09-19 10:30 ` David Bremner
2023-08-27 12:31 ` Kevin Boulain [this message]
2024-07-25 10:23 ` [PATCH v2 2/2] lib: thread-safe " David Bremner
2023-08-27 12:33 ` [PATCH " Kevin Boulain
2023-07-22 18:48 ` [PATCH 1/2] test: showcase thread-unsafe " David Bremner
2023-07-23 13:23 ` Michael J Gruber
2023-07-23 14:03 ` David Bremner
2023-08-06 10:05 ` Kevin Boulain
2023-08-06 12:01 ` David Bremner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://notmuchmail.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230827123103.22744-2-kevin@boula.in \
--to=kevin@boula.in \
--cc=notmuch@notmuchmail.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).