From 7d7c515b88ff2462dba86d3ae66ade6738afab29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=94=D0=B8=D0=BB=D1=8F=D0=BD=20=D0=9F=D0=B0=D0=BB=D0=B0?= =?UTF-8?q?=D1=83=D0=B7=D0=BE=D0=B2?= Date: Sun, 6 Aug 2023 15:56:19 +0200 Subject: [PATCH] Remove improved_mboxlist_sort leftovers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Remove option CYRUSDB_MBOXSORT • The database comparisson functions do not need a copy of the database handle, so simplify accordingly. • Delete unused functions bsearch_uncompare_mbox() and bsearch_compare_mbox() --- cassandane/Cassandane/Config.pm | 2 - cassandane/Cassandane/Cyrus/Prometheus.pm | 64 ------------- cassandane/Cassandane/Cyrus/Quota.pm | 90 ------------------- changes/next/deprecate_improved_mboxlist | 20 +++++ cunit/aaa-db.testc | 4 +- cunit/quota.testc | 3 +- doc/README.cyrusdb.md | 8 -- doc/internal/cyrusdb-api.html | 1 - doc/internal/improved_mboxlist_sort.html | 34 ------- doc/internal/index.html | 1 - docsrc/imap/developer/API/cyrusdb.rst | 8 -- docsrc/imap/developer/API/cyrusdb2.rst | 1 - .../thoughts/improved_mboxlist_sort.rst | 34 ------- .../download/release-notes/3.6/x/3.6.0.rst | 6 +- .../manpages/systemcommands/cyr_dbtool.rst | 6 +- imap/cyr_dbtool.c | 6 +- imap/quota.c | 5 +- imap/quota_db.c | 3 +- lib/bsearch.c | 38 -------- lib/bsearch.h | 4 - lib/cyrusdb.c | 2 +- lib/cyrusdb.h | 4 +- lib/cyrusdb_flat.c | 10 +-- lib/cyrusdb_quotalegacy.c | 32 +------ lib/cyrusdb_skiplist.c | 32 +++---- lib/cyrusdb_sql.c | 14 +-- lib/cyrusdb_twoskip.c | 27 ++---- lib/cyrusdb_zeroskip.c | 10 +-- lib/imapoptions | 12 +-- 29 files changed, 58 insertions(+), 423 deletions(-) create mode 100644 changes/next/deprecate_improved_mboxlist delete mode 100644 doc/internal/improved_mboxlist_sort.html delete mode 100644 docsrc/imap/developer/thoughts/improved_mboxlist_sort.rst diff --git a/cassandane/Cassandane/Config.pm b/cassandane/Cassandane/Config.pm index 5ca72584a9c..f7860368b14 100644 --- a/cassandane/Cassandane/Config.pm +++ b/cassandane/Cassandane/Config.pm @@ -108,8 +108,6 @@ sub default delete_mode => 'delayed', # for debugging - see cassandane.ini.example debug_command => '@prefix@/utils/gdbtramp %s %d', - # everyone should be running this - improved_mboxlist_sort => 'yes', # default changed, we want to be explicit about it unixhierarchysep => 'no', # let's hear all about it diff --git a/cassandane/Cassandane/Cyrus/Prometheus.pm b/cassandane/Cassandane/Cyrus/Prometheus.pm index 4cb3456f331..6c8791562f3 100644 --- a/cassandane/Cassandane/Cyrus/Prometheus.pm +++ b/cassandane/Cassandane/Cyrus/Prometheus.pm @@ -257,70 +257,6 @@ sub test_quota_commitments $self->assert_equals(10000, $report->{'cyrus_usage_quota_commitment'}->{'partition="p2",resource="STORAGE"'}->{value}); } -# tests for pathological quotaroot/partition subdivisions -sub test_quota_commitments_no_improved_mboxlist_sort - :min_version_3_1 :needs_component_httpd :Partition2 :NoStartInstances -{ - my ($self) = @_; - - $self->{instance}->{config}->set('improved_mboxlist_sort', 'no'); - $self->_start_instances(); - - my $admintalk = $self->{adminstore}->get_client(); - - my $inbox = 'user.cassandane'; # allocate top level quota here - my $child = "$inbox.child"; - my $gchild1 = "$child.cat"; # we'll stick this one on a sep part - my $gchild2 = "$child.dog"; # give this one its own quota - my $gchild3 = "$child.sheep"; # normal, but sorts after weird ones - my $ggchild1 = "$gchild1.manx"; # and give this one its own quota - my $ggchild2 = "$gchild1.siamese"; # and this one back on def part - my $interm = "$inbox.foo.bar.baz"; # contains intermediate folders - my $inbox2 = 'user.cassandane-child'; # hyphen! own quota - - # make some folders - foreach my $f ($child, $gchild1, $gchild2, $gchild3, $ggchild1, - $ggchild2, $interm, $inbox2) { - $admintalk->create($f); - $self->assert_str_equals('ok', - $admintalk->get_last_completion_response()); - } - - # stick one of them on a different partition - $admintalk->rename($gchild1, $gchild1, 'p2'); - $self->assert_str_equals('ok', $admintalk->get_last_completion_response()); - - # but not one of its children - $admintalk->rename($ggchild2, $ggchild2, 'default'); - $self->assert_str_equals('ok', $admintalk->get_last_completion_response()); - - # create a mess of quotas - $admintalk->setquota($inbox, '(STORAGE 8000)'); - $self->assert_str_equals('ok', $admintalk->get_last_completion_response()); - $admintalk->setquota($gchild2, '(STORAGE 4000)'); - $self->assert_str_equals('ok', $admintalk->get_last_completion_response()); - $admintalk->setquota($ggchild1, '(STORAGE 2000)'); - $self->assert_str_equals('ok', $admintalk->get_last_completion_response()); - $admintalk->setquota($inbox2, '(STORAGE 1000)'); - $self->assert_str_equals('ok', $admintalk->get_last_completion_response()); - - $admintalk->logout(); - - sleep 3; - - my $response = $self->http_report(); - $self->assert($response->{success}); - - my $report = parse_report($response->{content}); - $self->assert(scalar keys %{$report}); - - # now we expect default partition to have 8000 + 4000 +1000 committed - $self->assert_equals(13000, $report->{'cyrus_usage_quota_commitment'}->{'partition="default",resource="STORAGE"'}->{value}); - - # and p2 partition to have 8000 + 2000 committed - $self->assert_equals(10000, $report->{'cyrus_usage_quota_commitment'}->{'partition="p2",resource="STORAGE"'}->{value}); -} - sub test_shared_mailbox_namespaces :min_version_3_1 :needs_component_httpd { diff --git a/cassandane/Cassandane/Cyrus/Quota.pm b/cassandane/Cassandane/Cyrus/Quota.pm index e7f3b6a088c..e01d9dc3c6a 100644 --- a/cassandane/Cassandane/Cyrus/Quota.pm +++ b/cassandane/Cassandane/Cyrus/Quota.pm @@ -1121,96 +1121,6 @@ sub test_quota_d $self->assert_does_not_match(qr{dave\@qux.com}, $content); } -# https://github.com/cyrusimap/cyrus-imapd/issues/2877 -sub test_quota_f_no_improved_mboxlist_sort - :unixHierarchySep :AltNamespace :VirtDomains :NoStartInstances -{ - my ($self) = @_; - - my $user = 'user1@example.com'; - my @otherusers = ( - 'user0@example.com', - 'user1-z@example.com', - 'user2@example.com', - ); - - $self->{instance}->{config}->set('improved_mboxlist_sort', 'no'); - $self->_start_instances(); - - my $admintalk = $self->{adminstore}->get_client(); - $admintalk->create("user/$user"); - $self->assert_str_equals('ok', - $admintalk->get_last_completion_response()); - $admintalk->setacl("user/$user", $user, 'lrswipkxtecdan'); - $self->assert_str_equals('ok', - $admintalk->get_last_completion_response()); - - xlog $self, "set ourselves a basic usage quota"; - $self->_set_limits( - quotaroot => "user/$user", - storage => 100000, - message => 50000, - $res_annot_storage => 10000, - ); - $self->_check_usages( - quotaroot => "user/$user", - storage => 0, - message => 0, - $res_annot_storage => 0, - ); - - # create some other users to tickle sort-order issues? - foreach my $x (@otherusers) { - $admintalk->create("user/$x"); - $self->_set_limits( - quotaroot => "user/$x", - storage => 100000, - message => 50000, - $res_annot_storage => 10000, - ); - } - - my $svc = $self->{instance}->get_service('imap'); - my $userstore = $svc->create_store(username => $user); - my $usertalk = $userstore->get_client(); - - foreach my $submbox ('Drafts', 'Junk', 'Sent', 'Trash') { - xlog $self, "creating $submbox..."; - $usertalk->create($submbox); - $self->assert_str_equals('ok', - $usertalk->get_last_completion_response()); - } - - $usertalk->list("", "*"); - - foreach my $mbox (qw(INBOX Drafts Sent Junk Trash)) { - $usertalk->select($mbox); - foreach (1..3) { - $self->make_message("msg $_ in $mbox", store => $userstore); - } - } - - xlog $self, "run quota -d"; - $self->{instance}->run_command({ cyrus => 1 }, - 'quota', '-d', 'example.com'); - - xlog $self, "run quota -d -f"; - my $outfile = $self->{instance}->{basedir} . '/quota.out'; - my @data = $self->{instance}->run_command({ - cyrus => 1, - redirects => { - stderr => $outfile, - stdout => $outfile, - }, - }, 'quota', '-f', '-d', 'example.com'); - - my $str = slurp_file($outfile); - xlog $self, $str; - - #example.com!user.user1.Junk: quota root example.com!user.user1 --> (none) - $self->assert_does_not_match(qr{ quota root \S+ --> \(none\)}, $str); -} - sub test_quota_f_unixhs :UnixHierarchySep { diff --git a/changes/next/deprecate_improved_mboxlist b/changes/next/deprecate_improved_mboxlist new file mode 100644 index 00000000000..8f3e2e89227 --- /dev/null +++ b/changes/next/deprecate_improved_mboxlist @@ -0,0 +1,20 @@ +Description: + +The improved_mboxlist_sort option had no effect since v3.6. It is now deprecated. + +cyr_dbtool option -M/--improved-mboxlist-sort is removed. + + +Config changes: + +improved_mboxlist_sort is now deprecated. + + +Upgrade instructions: + +Remove improved_mboxlist_sort from imapd.conf. It has no effect and is now deprecated. + + +GitHub issue: + +https://github.com/cyrusimap/cyrus-imapd/pull/4559 diff --git a/cunit/aaa-db.testc b/cunit/aaa-db.testc index b8abdbc0393..1e1b8a89fd7 100644 --- a/cunit/aaa-db.testc +++ b/cunit/aaa-db.testc @@ -799,9 +799,9 @@ static void test_mboxlist(void) r = cyrusdb_close(db); CU_ASSERT_EQUAL(r, CYRUSDB_OK); - /* try again with mboxlist sort */ + /* try again. This test used CYRUSDB_MBOXSORT. */ - r = cyrusdb_open(backend, filename2, CYRUSDB_CREATE | CYRUSDB_MBOXSORT, &db); + r = cyrusdb_open(backend, filename2, CYRUSDB_CREATE, &db); CU_ASSERT_EQUAL(r, CYRUSDB_OK); CU_ASSERT_PTR_NOT_NULL(db); diff --git a/cunit/quota.testc b/cunit/quota.testc index 403f2ec29fc..d2d1ee3591f 100644 --- a/cunit/quota.testc +++ b/cunit/quota.testc @@ -734,8 +734,7 @@ static void notest_foreach(void) #endif /* - * TODO: should test for quota_foreach() iteration order, - * with and without IMAPOPT_IMPROVED_MBOXLIST_SORT set. + * TODO: should test for quota_foreach() iteration order. * There is code that depends on it, e.g. for quota -f. */ diff --git a/doc/README.cyrusdb.md b/doc/README.cyrusdb.md index d2d630324e8..9c89dedfe49 100644 --- a/doc/README.cyrusdb.md +++ b/doc/README.cyrusdb.md @@ -85,14 +85,6 @@ if (!r) return mydb; // if (r == CYRUSDB_OK) { ... } * `CYRUSDB_CREATE` - if the named database doesn't exist, create a blank database. -* `CYRUSDB_MBOXSORT` - use the abomination called - `improved_mboxlist_sort` which re-orders a couple of characters to - allow "foo.bar" to sort before "foo bar", for perfectly good - reasons, but we're going to fix it a better way. Not every engine - supports arbitrary collation, and if many engines corrupt horribly - if the same database is opened with different choices for this flag. - Ouch. - * `CYRUSDB_CONVERT` - if set and the database fails to open, attempt a magic detection on the file content and try to convert the database to the requested backend type before opening it. In-place upgrades! diff --git a/doc/internal/cyrusdb-api.html b/doc/internal/cyrusdb-api.html index e8fe6aab76e..9e606a6e4ff 100644 --- a/doc/internal/cyrusdb-api.html +++ b/doc/internal/cyrusdb-api.html @@ -154,7 +154,6 @@

cyrusdb_open(const char *backend, const char *fname, int flags,

Flags:

Errors:

diff --git a/doc/internal/improved_mboxlist_sort.html b/doc/internal/improved_mboxlist_sort.html deleted file mode 100644 index 5e1acd9936c..00000000000 --- a/doc/internal/improved_mboxlist_sort.html +++ /dev/null @@ -1,34 +0,0 @@ - - - -Enabling improved_mboxlist_sort - - -

Enabling improved_mboxlist_sort

- -

You can't enable and disable improved_mboxlist_sort on a live -system. You need to dump and load the necessary database after -stopping and before starting the master process.

- -

Dumping the mailboxes.db file

-
-ctl_mboxlist -d > /var/tmp/mailboxes.txt
-ctl_mboxlist -u < /var/tmp/mailboxes.txt
-
- -

If your subscription databases are not in flat files you need to do -something similar. Each user will have his own subscription file. Do -the following for each subscription file.

- -
-cyr_dbtool -C $file skiplist show > $file.TXT
-cyr_dbtool -n $file skiplist set < $file.TXT
-
- -

The above fragments will overwrite the original file. So you could -redirect to a temporary file and overwrite the database if the import -succeeds.

- - - diff --git a/doc/internal/index.html b/doc/internal/index.html index 69c65652a8e..bb7fe41674f 100644 --- a/doc/internal/index.html +++ b/doc/internal/index.html @@ -44,7 +44,6 @@

The Cyrus IMAP Server Distribution

  • Special Characters
    • -
  • Enabling improved_mboxlist_sort
    • diff --git a/docsrc/imap/developer/API/cyrusdb.rst b/docsrc/imap/developer/API/cyrusdb.rst index 7404eec3cb8..924b551c509 100644 --- a/docsrc/imap/developer/API/cyrusdb.rst +++ b/docsrc/imap/developer/API/cyrusdb.rst @@ -196,13 +196,6 @@ Accepted flags: CYRUSDB_CREATE - if the named database doesn't exist, create a blank database. -CYRUSDB_MBOXSORT - use the abomination called improved_mboxlist_sort -which re-orders a couple of characters to allow "foo.bar" to sort -before "foo bar", for perfectly good reasons, but we're going to fix it -a better way. Not every engine supports arbitrary collation, and if -many engines corrupt horribly if the same database is opened with -different choices for this flag. Ouch. - CYRUSDB_CONVERT - if set and the database fails to open, attempt a magic detection on the file content and try to convert the database to the requested backend type before opening it. In-place upgrades! If @@ -487,7 +480,6 @@ returns an opaque database structure Flags: - CYRUSDB\_CREATE - create the database if it doesn't exist -- CYRUSDB\_MBOXSORT - sort '.' first, so folder listing is correct Errors: diff --git a/docsrc/imap/developer/API/cyrusdb2.rst b/docsrc/imap/developer/API/cyrusdb2.rst index 42760e3cd68..18f8525dcff 100644 --- a/docsrc/imap/developer/API/cyrusdb2.rst +++ b/docsrc/imap/developer/API/cyrusdb2.rst @@ -143,7 +143,6 @@ returns an opaque database structure Flags: - CYRUSDB\_CREATE - create the database if it doesn't exist -- CYRUSDB\_MBOXSORT - sort '.' first, so folder listing is correct Errors: diff --git a/docsrc/imap/developer/thoughts/improved_mboxlist_sort.rst b/docsrc/imap/developer/thoughts/improved_mboxlist_sort.rst deleted file mode 100644 index 7893c78eec9..00000000000 --- a/docsrc/imap/developer/thoughts/improved_mboxlist_sort.rst +++ /dev/null @@ -1,34 +0,0 @@ -.. Note: This document was converted from the original by Nic Bernstein - (Onlight). Any formatting mistakes are my fault and not the - original author's. Converted via the pandoc tool from HTML. - -.. _enabling improved mboxlist sort: - -Enabling improved_mboxlist_sort -=============================== - -You can't enable and disable ``improved_mboxlist_sort`` on a live -system. You need to dump and load the necessary database after stopping -and before starting the master process. Rename the original mailboxes.db -out of the way between dumping the old and loading the new. - -Dumping the mailboxes.db file - -:: - - ctl_mboxlist -d > /var/tmp/mailboxes.txt - mv mailboxes.db mailboxes.db.orig - ctl_mboxlist -u < /var/tmp/mailboxes.txt - -If your subscription databases are not in flat files you need to do -something similar. Each user will have his own subscription file. Do the -following for each subscription file. - -:: - - cyr_dbtool -C $file skiplist show > $file.TXT - cyr_dbtool -n $file skiplist set < $file.TXT - -The above fragments will overwrite the original file. So you could -redirect to a temporary file and overwrite the database if the import -succeeds. diff --git a/docsrc/imap/download/release-notes/3.6/x/3.6.0.rst b/docsrc/imap/download/release-notes/3.6/x/3.6.0.rst index 173ae67f19c..7a6346c9172 100644 --- a/docsrc/imap/download/release-notes/3.6/x/3.6.0.rst +++ b/docsrc/imap/download/release-notes/3.6/x/3.6.0.rst @@ -132,6 +132,7 @@ change during the process. * The `reverseuniqueids` :cyrusman:`imapd.conf(5)` option is now deprecated and unused. Reverse UNIQUEID records are now standard and cannot be turned off. +* The `improved_mboxlist_sort` :cyrusman:`imapd.conf(5)` option does nothing. Security fixes ============== @@ -150,10 +151,7 @@ Significant bugfixes XML element instead of text (thanks Дилян Палаузов) * Fixed :issue:`3896`: the `-d` (dump) and `-u` (undump) options to :cyrusman:`ctl_mboxlist(8)` now correctly dump and undump all fields in - mailboxes.db entries. The intermediary file format is now JSON. This change - makes it possible to follow the procedure - for switching `improved_mboxlist_sort` described in - :ref:`enabling improved mboxlist sort` + mailboxes.db entries. The intermediary file format is now JSON. * Fixed :issue:`4035`: `ctl_cyrusdb -r` now recovers from mailboxes.db records with missing uniqueids, instead of crashing. A new `-P` option to :cyrusman:`reconstruct(8)` enables repairing mailboxes whose header files diff --git a/docsrc/imap/reference/manpages/systemcommands/cyr_dbtool.rst b/docsrc/imap/reference/manpages/systemcommands/cyr_dbtool.rst index 14188d15a76..0995f56943b 100644 --- a/docsrc/imap/reference/manpages/systemcommands/cyr_dbtool.rst +++ b/docsrc/imap/reference/manpages/systemcommands/cyr_dbtool.rst @@ -16,7 +16,7 @@ Synopsis .. parsed-literal:: - **cyr_dbtool** [ **-C** *config-file* ] [ **-M** ] [ **-n** ] [ **-o** ] [ **-T** ] + **cyr_dbtool** [ **-C** *config-file* ] [ **-n** ] [ **-o** ] [ **-T** ] *db-file* *db-backend* *action* [ *key* ] [ *value* ] Description @@ -68,10 +68,6 @@ Options |cli-dash-c-text| -.. option:: -M, --improved-mboxlist-sort - - Uses improved MBOX list sort - .. option:: -n, --create Create the database file if it doesn't already exist. diff --git a/imap/cyr_dbtool.c b/imap/cyr_dbtool.c index 42e21e8a941..1f39d63ebdd 100644 --- a/imap/cyr_dbtool.c +++ b/imap/cyr_dbtool.c @@ -276,11 +276,10 @@ int main(int argc, char *argv[]) struct txn **tidp = NULL; /* keep this in alphabetical order */ - static const char short_options[] = "C:MTcnt"; + static const char short_options[] = "C:Tcnt"; static const struct option long_options[] = { /* n.b. no long option for -C */ - { "improved-mboxlist-sort", no_argument, NULL, 'M' }, { "use-transaction", no_argument, NULL, 'T' }, { "convert", no_argument, NULL, 'c' }, /* XXX undocumented */ { "create", no_argument, NULL, 'n' }, @@ -298,9 +297,6 @@ int main(int argc, char *argv[]) case 'c': db_flags |= CYRUSDB_CONVERT; break; - case 'M': /* use "improved_mboxlist_sort" */ - db_flags |= CYRUSDB_MBOXSORT; - break; case 'n': /* create new */ db_flags |= CYRUSDB_CREATE; break; diff --git a/imap/quota.c b/imap/quota.c index 7c91f0c0ade..5bc08d7ee53 100644 --- a/imap/quota.c +++ b/imap/quota.c @@ -108,7 +108,6 @@ static int fixquota_dopass(char *domain, char **roots, int nroots, mboxlist_cb *pass, int isuser); static int fixquota_fixroot(struct mailbox *mailbox, const char *root); static int fixquota_finish(int thisquota); -static int (*compar)(const char *s1, const char *s2); #define QUOTAGROW 300 @@ -201,8 +200,6 @@ int main(int argc,char **argv) fatal(error_message(r), EX_CONFIG); } - compar = strcmp; - /* * Lock mailbox list to prevent mailbox creation/deletion * during work @@ -450,7 +447,7 @@ static int findroot(const char *name, int *thisquota) /* have we already passed the name, then there can * be no further matches */ - if (compar(root, name) > 0) + if (strcmp(root, name) > 0) break; /* is the mailbox within this root? */ diff --git a/imap/quota_db.c b/imap/quota_db.c index bede6dfe5f1..be0e3a72202 100644 --- a/imap/quota_db.c +++ b/imap/quota_db.c @@ -711,7 +711,6 @@ EXPORTED void quotadb_open(const char *fname) { int ret; char *tofree = NULL; - int flags = CYRUSDB_CREATE; if (!fname) fname = config_getstring(IMAPOPT_QUOTA_DB_PATH); @@ -722,7 +721,7 @@ EXPORTED void quotadb_open(const char *fname) fname = tofree; } - ret = cyrusdb_open(QDB, fname, flags, &qdb); + ret = cyrusdb_open(QDB, fname, CYRUSDB_CREATE, &qdb); if (ret != 0) { syslog(LOG_ERR, "DBERROR: opening %s: %s", fname, cyrusdb_strerror(ret)); diff --git a/lib/bsearch.c b/lib/bsearch.c index 65edb0b360e..b929f036c6c 100644 --- a/lib/bsearch.c +++ b/lib/bsearch.c @@ -177,25 +177,6 @@ HIDDEN int bsearch_mem_mbox(const char *word, return p - base + 1; } -EXPORTED int bsearch_compare_mbox(const char *s1, const char *s2) -{ - int cmp; - char c2; - - for (;;) { - if ((c2 = *s2) == 0) { - return (unsigned char)*s1; - } - cmp = TOCOMPARE(*s1) - TOCOMPARE(c2); - if (cmp) return cmp; - if (TOCOMPARE(c2) == TOCOMPARE('\t')) { - return 0; - } - s1++; - s2++; - } -} - HIDDEN int bsearch_ncompare_mbox(const char *s1, int l1, const char *s2, int l2) { int min = l1 < l2 ? l1 : l2; @@ -214,25 +195,6 @@ HIDDEN int bsearch_ncompare_mbox(const char *s1, int l1, const char *s2, int l2) } } -HIDDEN int bsearch_uncompare_mbox(const unsigned char *s1, size_t l1, - const unsigned char *s2, size_t l2) -{ - ssize_t min = l1 < l2 ? l1 : l2; - int cmp = 0; - - while (min-- > 0 && (cmp = TOCOMPARE(*s1) - TOCOMPARE(*s2)) == 0) { - s1++; - s2++; - } - if (min >= 0) { - return cmp; - } else { - if (l2 > l1) return -1; - else if (l1 > l2) return 1; - else return 0; - } -} - HIDDEN int bsearch_memtree_mbox(const unsigned char *s1, size_t l1, const unsigned char *s2, size_t l2) { diff --git a/lib/bsearch.h b/lib/bsearch.h index 157fb20712a..bf5fbd6f0bc 100644 --- a/lib/bsearch.h +++ b/lib/bsearch.h @@ -48,11 +48,7 @@ extern int bsearch_mem_mbox(const char *word, unsigned long hint, unsigned long *linelenp); -extern int bsearch_compare_mbox(const char *s1, const char *s2); - extern int bsearch_ncompare_mbox(const char *s1, int l1, const char *s2, int l2); -extern int bsearch_uncompare_mbox(const unsigned char *s1, size_t l1, - const unsigned char *s2, size_t l2); extern int bsearch_memtree_mbox(const unsigned char *s1, size_t l1, const unsigned char *s2, size_t l2); diff --git a/lib/cyrusdb.c b/lib/cyrusdb.c index 077ee5b1551..6d3f80ff6d2 100644 --- a/lib/cyrusdb.c +++ b/lib/cyrusdb.c @@ -383,7 +383,7 @@ EXPORTED int cyrusdb_compar(struct db *db, const char *a, int alen, const char *b, int blen) { - return db->backend->compar(db->engine, a, alen, b, blen); + return db->backend->compar(a, alen, b, blen); } /**********************************************/ diff --git a/lib/cyrusdb.h b/lib/cyrusdb.h index cce0dd1e006..978d74956da 100644 --- a/lib/cyrusdb.h +++ b/lib/cyrusdb.h @@ -72,7 +72,6 @@ enum cyrusdb_dbflags { enum cyrusdb_openflags { CYRUSDB_CREATE = 0x01, /* Create the database if not existant */ - CYRUSDB_MBOXSORT = 0x02, /* Use mailbox sort order ('.' sorts 1st) */ CYRUSDB_CONVERT = 0x04, /* Convert to the named format if not already */ CYRUSDB_NOCOMPACT = 0x08, /* Don't run any database compaction routines */ CYRUSDB_SHARED = 0x10, /* Open in shared lock mode */ @@ -221,8 +220,7 @@ struct cyrusdb_backend { int (*dump)(struct dbengine *db, int detail); int (*consistent)(struct dbengine *db); int (*repack)(struct dbengine *db); - int (*compar)(struct dbengine *db, const char *s1, int l1, - const char *s2, int l2); + int (*compar)(const char *s1, int l1, const char *s2, int l2); }; extern int cyrusdb_copyfile(const char *srcname, const char *dstname); diff --git a/lib/cyrusdb_flat.c b/lib/cyrusdb_flat.c index 5c714d51025..547818006ad 100644 --- a/lib/cyrusdb_flat.c +++ b/lib/cyrusdb_flat.c @@ -852,14 +852,6 @@ static int commit_txn(struct dbengine *db, struct txn *tid) return r; } -/* flat database is always mbox sort order */ -static int mycompar(struct dbengine *db __attribute__((unused)), - const char *a, int alen, - const char *b, int blen) -{ - return bsearch_ncompare_mbox(a, alen, b, blen); -} - EXPORTED struct cyrusdb_backend cyrusdb_flat = { "flat", /* name */ @@ -888,5 +880,5 @@ EXPORTED struct cyrusdb_backend cyrusdb_flat = NULL, NULL, NULL, - &mycompar + &bsearch_ncompare_mbox }; diff --git a/lib/cyrusdb_quotalegacy.c b/lib/cyrusdb_quotalegacy.c index 0a3fe480ecc..a0b56a2c0bd 100644 --- a/lib/cyrusdb_quotalegacy.c +++ b/lib/cyrusdb_quotalegacy.c @@ -124,14 +124,10 @@ struct dbengine { char *data; /* allocated buffer for fetched data */ struct txn txn; /* transaction associated with this db handle */ - - /* sorting function */ - int (*compar) (const void *s1, const void *s2); }; static int abort_txn(struct dbengine *db __attribute__((unused)), struct txn *tid); static int compar_qr(const void *v1, const void *v2); -static int compar_qr_mbox(const void *v1, const void *v2); /* hash the prefix - either with or without 'user.' part */ static char name_to_hashchar(const char *name, int isprefix) @@ -349,8 +345,6 @@ static int myopen(const char *fname, int flags, struct dbengine **ret, struct tx return CYRUSDB_IOERROR; } - db->compar = (flags & CYRUSDB_MBOXSORT) ? compar_qr_mbox : compar_qr; - *ret = db; if (mytid) { @@ -523,17 +517,6 @@ static int compar_qr(const void *v1, const void *v2) return strcmp(qr1, qr2); } -static int compar_qr_mbox(const void *v1, const void *v2) -{ - const char *qr1, *qr2; - char qrbuf1[MAX_QUOTA_PATH+1], qrbuf2[MAX_QUOTA_PATH+1]; - - qr1 = path_to_qr(*((const char **) v1), qrbuf1); - qr2 = path_to_qr(*((const char **) v2), qrbuf2); - - return bsearch_compare_mbox(qr1, qr2); -} - static void scan_qr_dir(char *quota_path, const char *prefix, strarray_t *pathbuf) { @@ -658,7 +641,7 @@ static int foreach(struct dbengine *db, /* sort the quotaroots (ignoring paths) */ if (pathbuf.data) - qsort(pathbuf.data, pathbuf.count, sizeof(char *), db->compar); + qsort(pathbuf.data, pathbuf.count, sizeof(char *), compar_qr); for (i = 0; i < pathbuf.count; i++) { const char *data, *key; @@ -890,17 +873,6 @@ static int abort_txn(struct dbengine *db __attribute__((unused)), struct txn *ti return tid->result; } -/* quotalegacy gets compar set at startup, but it's not the same */ -static int mycompar(struct dbengine *db, const char *a, int alen, - const char*b, int blen) -{ - if (db->compar == compar_qr_mbox) - return bsearch_ncompare_mbox(a, alen, b, blen); - else - return bsearch_ncompare_raw(a, alen, b, blen); -} - - HIDDEN struct cyrusdb_backend cyrusdb_quotalegacy = { "quotalegacy", /* name */ @@ -929,5 +901,5 @@ HIDDEN struct cyrusdb_backend cyrusdb_quotalegacy = NULL, NULL, NULL, - &mycompar + &bsearch_ncompare_raw }; diff --git a/lib/cyrusdb_skiplist.c b/lib/cyrusdb_skiplist.c index 6601a0dc1bf..65771fad96a 100644 --- a/lib/cyrusdb_skiplist.c +++ b/lib/cyrusdb_skiplist.c @@ -176,9 +176,6 @@ struct dbengine { int open_flags; struct txn *current_txn; struct timeval starttime; - - /* comparator function to use for sorting */ - int (*compar) (const char *s1, int l1, const char *s2, int l2); }; struct db_list { @@ -875,7 +872,6 @@ static int myopen(const char *fname, int flags, struct dbengine **ret, struct tx db = (struct dbengine *) xzmalloc(sizeof(struct dbengine)); db->fd = -1; db->fname = xstrdup(fname); - db->compar = (flags & CYRUSDB_MBOXSORT) ? bsearch_ncompare_mbox : compare_signed; db->open_flags = (flags & ~CYRUSDB_CREATE); db->fd = open(fname, O_RDWR, 0644); @@ -1044,7 +1040,7 @@ static const char *find_node(struct dbengine *db, for (i = db->curlevel - 1; i >= 0; i--) { while ((offset = FORWARD(ptr, i)) && - db->compar(KEY(db->map_base + offset), KEYLEN(db->map_base + offset), + compare_signed(KEY(db->map_base + offset), KEYLEN(db->map_base + offset), key, keylen) < 0) { /* move forward at level 'i' */ ptr = db->map_base + offset; @@ -1093,7 +1089,7 @@ static int myfetch(struct dbengine *db, ptr = find_node(db, key, keylen, 0); - if (ptr == db->map_base || db->compar(KEY(ptr), KEYLEN(ptr), key, keylen)) { + if (ptr == db->map_base || compare_signed(KEY(ptr), KEYLEN(ptr), key, keylen)) { /* failed to find key/keylen */ r = CYRUSDB_NOTFOUND; } else { @@ -1158,7 +1154,7 @@ static int myforeach(struct dbengine *db, while (ptr != db->map_base) { /* does it match prefix? */ if (KEYLEN(ptr) < (uint32_t) prefixlen) break; - if (prefixlen && db->compar(KEY(ptr), prefixlen, prefix, prefixlen)) break; + if (prefixlen && compare_signed(KEY(ptr), prefixlen, prefix, prefixlen)) break; if (!goodp || goodp(rock, KEY(ptr), KEYLEN(ptr), DATA(ptr), DATALEN(ptr))) { @@ -1299,7 +1295,7 @@ static int mystore(struct dbengine *db, newoffset = tid->logend; ptr = find_node(db, key, keylen, updateoffsets); if (ptr != db->map_base && - !db->compar(KEY(ptr), KEYLEN(ptr), key, keylen)) { + !compare_signed(KEY(ptr), KEYLEN(ptr), key, keylen)) { if (!overwrite) { myabort(db, tid); /* releases lock */ @@ -1450,7 +1446,7 @@ static int mydelete(struct dbengine *db, ptr = find_node(db, key, keylen, updateoffsets); if (ptr != db->map_base && - !db->compar(KEY(ptr), KEYLEN(ptr), key, keylen)) { + !compare_signed(KEY(ptr), KEYLEN(ptr), key, keylen)) { /* gotcha */ offset = ptr - db->map_base; @@ -2014,11 +2010,11 @@ static int myconsistent(struct dbengine *db, struct txn *tid, int locked) const char *q = db->map_base + offset; int cmp; - cmp = db->compar(KEY(ptr), KEYLEN(ptr), KEY(q), KEYLEN(q)); + cmp = compare_signed(KEY(ptr), KEYLEN(ptr), KEY(q), KEYLEN(q)); if (cmp >= 0) { syslog(LOG_ERR, "skiplist inconsistent: %04X: ptr %d is %04X; " - "db->compar() = %d", + "compare_signed() = %d", (unsigned int) (ptr - db->map_base), i, offset, cmp); @@ -2273,7 +2269,7 @@ static int recovery(struct dbengine *db, int flags) if (TYPE(ptr) == ADD) { keyptr = find_node(db, KEY(ptr), KEYLEN(ptr), updateoffsets); if (keyptr == db->map_base || - db->compar(KEY(ptr), KEYLEN(ptr), KEY(keyptr), KEYLEN(keyptr))) { + compare_signed(KEY(ptr), KEYLEN(ptr), KEY(keyptr), KEYLEN(keyptr))) { /* didn't find exactly this node */ keyptr = NULL; } @@ -2284,7 +2280,7 @@ static int recovery(struct dbengine *db, int flags) p = db->map_base + myoff; keyptr = find_node(db, KEY(p), KEYLEN(p), updateoffsets); if (keyptr == db->map_base || - db->compar(KEY(p), KEYLEN(p), KEY(keyptr), KEYLEN(keyptr))) { + compare_signed(KEY(p), KEYLEN(p), KEY(keyptr), KEYLEN(keyptr))) { /* didn't find exactly this node */ keyptr = NULL; } @@ -2456,14 +2452,6 @@ static int recovery(struct dbengine *db, int flags) return r; } -/* skiplist compar function is set at open */ -static int mycompar(struct dbengine *db, const char *a, int alen, - const char *b, int blen) -{ - return db->compar(a, alen, b, blen); -} - - EXPORTED struct cyrusdb_backend cyrusdb_skiplist = { "skiplist", /* name */ @@ -2492,5 +2480,5 @@ EXPORTED struct cyrusdb_backend cyrusdb_skiplist = &dump, &consistent, &mycheckpoint, - &mycompar + &compare_signed }; diff --git a/lib/cyrusdb_sql.c b/lib/cyrusdb_sql.c index b9dccafc619..00af9956f68 100644 --- a/lib/cyrusdb_sql.c +++ b/lib/cyrusdb_sql.c @@ -907,16 +907,6 @@ static int abort_txn(struct dbengine *db, struct txn *tid) return finish_txn(db, tid, 0); } -/* SQL databases have all sorts of evil collations - we can't - * make any assumptions though, so just assume raw */ -static int mycompar(struct dbengine *db __attribute__((unused)), - const char *a, int alen, - const char *b, int blen) -{ - (void)db; - return bsearch_ncompare_raw(a, alen, b, blen); -} - HIDDEN struct cyrusdb_backend cyrusdb_sql = { "sql", /* name */ @@ -945,5 +935,7 @@ HIDDEN struct cyrusdb_backend cyrusdb_sql = NULL, NULL, NULL, - &mycompar + /* SQL databases have all sorts of evil collations - we can't + * make any assumptions though, so just assume raw */ + &bsearch_ncompare_raw }; diff --git a/lib/cyrusdb_twoskip.c b/lib/cyrusdb_twoskip.c index 4cc4825c3ff..388270ae566 100644 --- a/lib/cyrusdb_twoskip.c +++ b/lib/cyrusdb_twoskip.c @@ -393,9 +393,7 @@ struct dbengine { int txn_num; struct txn *current_txn; - /* comparator function to use for sorting */ int open_flags; - int (*compar) (const char *s1, int l1, const char *s2, int l2); }; struct db_list { @@ -963,7 +961,7 @@ static int relocate(struct dbengine *db) if (newrecord.offset) { assert(newrecord.level >= level); - cmp = db->compar(KEY(db, &newrecord), newrecord.keylen, + cmp = bsearch_ncompare_raw(KEY(db, &newrecord), newrecord.keylen, loc->keybuf.s, loc->keybuf.len); /* not there? stay at this level */ @@ -1010,7 +1008,7 @@ static int find_loc(struct dbengine *db, const char *key, size_t keylen) /* can we special case advance? */ if (keylen && loc->end == db->end && loc->generation == db->header.generation) { - cmp = db->compar(KEY(db, &loc->record), loc->record.keylen, + cmp = bsearch_ncompare_raw(KEY(db, &loc->record), loc->record.keylen, loc->keybuf.s, loc->keybuf.len); /* same place, and was exact. Otherwise we're going back, * and the reverse pointers are no longer valid... */ @@ -1034,7 +1032,7 @@ static int find_loc(struct dbengine *db, const char *key, size_t keylen) } /* now where is THIS record? */ - cmp = db->compar(KEY(db, &newrecord), newrecord.keylen, + cmp = bsearch_ncompare_raw(KEY(db, &newrecord), newrecord.keylen, loc->keybuf.s, loc->keybuf.len); /* exact match? */ @@ -1356,8 +1354,6 @@ static int opendb(const char *fname, int flags, struct dbengine **ret, struct tx mappedfile_flags |= MAPPEDFILE_CREATE; db->open_flags = flags & ~CYRUSDB_CREATE; - db->compar = (flags & CYRUSDB_MBOXSORT) ? bsearch_ncompare_mbox - : bsearch_ncompare_raw; r = mappedfile_open(&db->mf, fname, mappedfile_flags); if (r) { @@ -1649,7 +1645,7 @@ static int myforeach(struct dbengine *db, /* does it match prefix? */ if (prefixlen) { if (db->loc.record.keylen < prefixlen) break; - if (db->compar(KEY(db, &db->loc.record), prefixlen, prefix, prefixlen)) break; + if (bsearch_ncompare_raw(KEY(db, &db->loc.record), prefixlen, prefix, prefixlen)) break; } val = VAL(db, &db->loc.record); @@ -1744,7 +1740,7 @@ static int skipwrite(struct dbengine *db, if (!data) return delete_here(db); if (!force) return CYRUSDB_EXISTS; /* unchanged? Save the IO */ - if (!db->compar(data, datalen, + if (!bsearch_ncompare_raw(data, datalen, VAL(db, &db->loc.record), db->loc.record.vallen)) return 0; @@ -2172,7 +2168,7 @@ static int myconsistent(struct dbengine *db, struct txn *tid) continue; } - cmp = db->compar(KEY(db, &record), record.keylen, + cmp = bsearch_ncompare_raw(KEY(db, &record), record.keylen, KEY(db, &prevrecord), prevrecord.keylen); if (cmp <= 0) { xsyslog(LOG_ERR, "DBERROR: twoskip out of order", @@ -2426,7 +2422,7 @@ static int recovery1(struct dbengine *db, int *count) continue; } - cmp = db->compar(KEY(db, &record), record.keylen, + cmp = bsearch_ncompare_raw(KEY(db, &record), record.keylen, KEY(db, &prevrecord), prevrecord.keylen); if (cmp <= 0) { xsyslog(LOG_ERR, "DBERROR: twoskip out of order", @@ -2587,13 +2583,6 @@ static int delete(struct dbengine *db, return mystore(db, key, keylen, NULL, 0, tid, force); } -/* twoskip compar function is set at open */ -static int mycompar(struct dbengine *db, const char *a, int alen, - const char *b, int blen) -{ - return db->compar(a, alen, b, blen); -} - HIDDEN struct cyrusdb_backend cyrusdb_twoskip = { "twoskip", /* name */ @@ -2622,5 +2611,5 @@ HIDDEN struct cyrusdb_backend cyrusdb_twoskip = &dump, &consistent, &mycheckpoint, - &mycompar + &bsearch_ncompare_raw }; diff --git a/lib/cyrusdb_zeroskip.c b/lib/cyrusdb_zeroskip.c index 95bdef3df65..9144ec023af 100644 --- a/lib/cyrusdb_zeroskip.c +++ b/lib/cyrusdb_zeroskip.c @@ -156,21 +156,13 @@ static int cyrusdb_zeroskip_open(const char *fname, struct dbengine *dbe; int r = CYRUSDB_OK; int zsdbflags = MODE_RDWR; - zsdb_cmp_fn dbcmpfn = NULL; - memtree_search_cb_t btcmpfn = NULL; dbe = (struct dbengine *) xzmalloc(sizeof(struct dbengine)); if (flags & CYRUSDB_CREATE) zsdbflags = MODE_CREATE; - if (flags & CYRUSDB_MBOXSORT) { - zsdbflags |= MODE_CUSTOMSEARCH; - dbcmpfn = bsearch_uncompare_mbox; - btcmpfn = memtree_memcmp_mbox; - } - - if (zsdb_init(&dbe->db, dbcmpfn, btcmpfn) != ZS_OK) { + if (zsdb_init(&dbe->db, NULL, NULL) != ZS_OK) { r = CYRUSDB_IOERROR; goto done; } diff --git a/lib/imapoptions b/lib/imapoptions index fd9d4abe461..575ff511d08 100644 --- a/lib/imapoptions +++ b/lib/imapoptions @@ -1205,16 +1205,8 @@ Blank lines and lines beginning with ``#'' are ignored. configuration. If the path to the file is not absolute, CYRUS_PATH is prepended. */ -{ "improved_mboxlist_sort", 0, SWITCH, "2.3.17" } -/* If enabled, a special comparator will be used which will correctly - sort mailbox names that contain characters such as ' ' and '-'. -.PP - Note that this option SHOULD NOT be changed on a live system. The - mailboxes database should be dumped (ctl_mboxlist) before the - option is changed, removed, and then undumped after changing the - option. When not using flat files for the subscriptions databases - the same has to be done (cyr_dbtool) for each subscription database - See improved_mboxlist_sort.html.*/ +{ "improved_mboxlist_sort", 0, SWITCH, "UNRELEASED", "UNRELEASED" } +/* Not used anymore. */ { "jmap_emailsearch_db_path", NULL, STRING, "3.1.6", "3.6.0" } /* The absolute path to the JMAP email search cache file. If not