On Sun, Apr 10 2016, Jani Nikula <jani@nikula.org> 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 <cworth@cworth.org> > 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 <<EOF > 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