From: Tomi Ollila Date: Mon, 11 Apr 2016 06:44:49 +0000 (+0300) Subject: Re: [PATCH 2/3] lib: fix handling of one character long directory names at top level X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=2b0ef844df6c651c61906c31db8eb4e3b1d219c6;p=notmuch-archives.git Re: [PATCH 2/3] lib: fix handling of one character long directory names at top level --- diff --git a/c6/ed78772b7061109ca6c9199a64ae315698eb8b b/c6/ed78772b7061109ca6c9199a64ae315698eb8b new file mode 100644 index 000000000..3c67e7d63 --- /dev/null +++ b/c6/ed78772b7061109ca6c9199a64ae315698eb8b @@ -0,0 +1,151 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by arlo.cworth.org (Postfix) with ESMTP id A91D26DE0350 + for ; Sun, 10 Apr 2016 23:44:56 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at cworth.org +X-Spam-Flag: NO +X-Spam-Score: 0.62 +X-Spam-Level: +X-Spam-Status: No, score=0.62 tagged_above=-999 required=5 tests=[AWL=-0.032, + SPF_NEUTRAL=0.652] autolearn=disabled +Received: from arlo.cworth.org ([127.0.0.1]) + by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id GiHOHFxr9M74 for ; + Sun, 10 Apr 2016 23:44:48 -0700 (PDT) +Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34]) + by arlo.cworth.org (Postfix) with ESMTP id E4D186DE0317 + for ; Sun, 10 Apr 2016 23:44:47 -0700 (PDT) +Received: from guru.guru-group.fi (localhost [IPv6:::1]) + by guru.guru-group.fi (Postfix) with ESMTP id 28E90100090; + Mon, 11 Apr 2016 09:44:50 +0300 (EEST) +From: Tomi Ollila +To: Jani Nikula , notmuch@notmuchmail.org +Subject: Re: [PATCH 2/3] lib: fix handling of one character long directory + names at top level +In-Reply-To: <1460317403-13714-2-git-send-email-jani@nikula.org> +References: <1460317403-13714-1-git-send-email-jani@nikula.org> + <1460317403-13714-2-git-send-email-jani@nikula.org> +User-Agent: Notmuch/0.21+99~g7cbc880 (http://notmuchmail.org) Emacs/24.3.1 + (x86_64-unknown-linux-gnu) +X-Face: HhBM'cA~ +MIME-Version: 1.0 +Content-Type: text/plain +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.20 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Mon, 11 Apr 2016 06:44:56 -0000 + +On Sun, Apr 10 2016, Jani Nikula wrote: + +> The code to skip multiple slashes in _notmuch_database_split_path() +> skips back one character too much. This is compensated by a +1 in the +> length parameter to the strndup() call. Mostly this works fine, but if +> the path is to a file under a top level directory with one character +> long name, the directory part is mistaken to be part of the file name +> (slash == path in code). The returned directory name will be the empty +> string and the basename will be the full path, breaking the indexing +> logic in notmuch new. +> +> Fix the multiple slash skipping to keep the slash variable pointing at +> the last slash, and adjust strndup() accordingly. + +Good work. Changes look good. Tests pass... + +... I also debugged this yesterday with an extra debug print: + ++++ b/notmuch-new.c +@@ -382,2 +382,3 @@ add_files (notmuch_database_t *notmuch, + status = notmuch_database_get_directory (notmuch, path, &directory); ++ printf("(Z) %s %p %d\n", path, directory, status); + if (status) { + +Without these changes the %p printing directory value never changed from +(nil) to ... non-nil with one-character directory names (meaning directory +was found in database), after this patch the directory value worked like +with longer directory names. + +Tomi + +> +> The bug was introduced in +> +> commit e890b0cf4011fd9fd77ebd87343379e4a778888b +> Author: Carl Worth +> Date: Sat Dec 19 13:20:26 2009 -0800 +> +> database: Store the parent ID for each directory document. +> +> just a little over two months after the initial commit in the Notmuch +> code history, making this the longest living bug in Notmuch to date. +> --- +> lib/database.cc | 4 ++-- +> test/T050-new.sh | 5 ----- +> 2 files changed, 2 insertions(+), 7 deletions(-) +> +> diff --git a/lib/database.cc b/lib/database.cc +> index 3b342f136a53..b8486f7d5271 100644 +> --- a/lib/database.cc +> +++ b/lib/database.cc +> @@ -1781,7 +1781,7 @@ _notmuch_database_split_path (void *ctx, +> +> /* Finally, skip multiple slashes. */ +> while (slash != path) { +> - if (*slash != '/') +> + if (*(slash - 1) != '/') +> break; +> +> --slash; +> @@ -1794,7 +1794,7 @@ _notmuch_database_split_path (void *ctx, +> *basename = path; +> } else { +> if (directory) +> - *directory = talloc_strndup (ctx, path, slash - path + 1); +> + *directory = talloc_strndup (ctx, path, slash - path); +> } +> +> return NOTMUCH_STATUS_SUCCESS; +> diff --git a/test/T050-new.sh b/test/T050-new.sh +> index 174715aa2781..53e02d22c383 100755 +> --- a/test/T050-new.sh +> +++ b/test/T050-new.sh +> @@ -170,7 +170,6 @@ test_expect_equal "$output" "(D) add_files, pass 3: queuing leftover directory $ +> No new mail. Removed 3 messages." +> +> test_begin_subtest "One character directory at top level" +> -test_subtest_known_broken +> +> generate_message [dir]=A +> generate_message [dir]=A/B +> @@ -179,10 +178,6 @@ generate_message [dir]=A/B/C +> output=$(NOTMUCH_NEW --debug) +> test_expect_equal "$output" "Added 3 new messages to the database." +> +> -# clean up after the broken test to not mess up other tests +> -rm -rf "${MAIL_DIR}"/A +> -NOTMUCH_NEW 2>&1 > /dev/null +> - +> test_begin_subtest "Support single-message mbox" +> cat > "${MAIL_DIR}"/mbox_file1 < From test_suite@notmuchmail.org Fri Jan 5 15:43:57 2001 +> -- +> 2.1.4 +> +> _______________________________________________ +> notmuch mailing list +> notmuch@notmuchmail.org +> https://notmuchmail.org/mailman/listinfo/notmuch