* [PATCH 1/2] test/dump-restore: new tests for empty files and leading comments/whitespace. @ 2012-12-26 19:36 david 2012-12-26 19:36 ` [PATCH 2/2] notmuch-restore: handle empty input file, leading blank lines and comments david 2013-01-07 2:53 ` [PATCH 1/2] test/dump-restore: new tests for empty files and leading comments/whitespace David Bremner 0 siblings, 2 replies; 6+ messages in thread From: david @ 2012-12-26 19:36 UTC (permalink / raw) To: notmuch; +Cc: David Bremner From: David Bremner <bremner@debian.org> Three of these are marked broken; the third is a regression test, since it passes by virtue of batch-tag being the default input format. --- test/dump-restore | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/test/dump-restore b/test/dump-restore index 6a989b6..c2ddb92 100755 --- a/test/dump-restore +++ b/test/dump-restore @@ -136,6 +136,48 @@ notmuch dump --format=batch-tag > BACKUP notmuch tag +"$tag1" +"$tag2" +"$tag3" -inbox -unread "*" +# initial segment of file used for several tests below. +cat <<EOF > comments-and-blanks +# this is a comment + +# next line has leading whitespace + + +EOF + +test_begin_subtest 'restoring empty file is not an error' +test_subtest_known_broken +notmuch restore < /dev/null 2>OUTPUT.$test_count +cp /dev/null EXPECTED +test_expect_equal_file EXPECTED OUTPUT.$test_count + +test_begin_subtest 'file of comments and blank lines is not an error' +test_subtest_known_broken +notmuch restore --input=comments-and-blanks +ret_val=$? +test_expect_equal "$ret_val" "0" + +cp comments-and-blanks leading-comments-blanks-batch-tag +echo "+some_tag -- id:yun1vjwegii.fsf@aiko.keithp.com" \ + >> leading-comments-blanks-batch-tag + +test_begin_subtest 'detect format=batch-tag with leading comments and blanks' +notmuch restore --input=leading-comments-blanks-batch-tag +notmuch search --output=tags id:yun1vjwegii.fsf@aiko.keithp.com > OUTPUT.$test_count +echo "some_tag" > EXPECTED +test_expect_equal_file EXPECTED OUTPUT.$test_count + +cp comments-and-blanks leading-comments-blanks-sup +echo "yun1vjwegii.fsf@aiko.keithp.com (another_tag)" \ + >> leading-comments-blanks-sup + +test_begin_subtest 'detect format=sup with leading comments and blanks' +test_subtest_known_broken +notmuch restore --input=leading-comments-blanks-sup +notmuch search --output=tags id:yun1vjwegii.fsf@aiko.keithp.com > OUTPUT.$test_count +echo "another_tag" > EXPECTED +test_expect_equal_file EXPECTED OUTPUT.$test_count + test_begin_subtest 'format=batch-tag, round trip with strange tags' notmuch dump --format=batch-tag > EXPECTED.$test_count notmuch dump --format=batch-tag | notmuch restore --format=batch-tag -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] notmuch-restore: handle empty input file, leading blank lines and comments. 2012-12-26 19:36 [PATCH 1/2] test/dump-restore: new tests for empty files and leading comments/whitespace david @ 2012-12-26 19:36 ` david 2013-01-05 22:11 ` Tomi Ollila 2013-01-07 2:53 ` [PATCH 1/2] test/dump-restore: new tests for empty files and leading comments/whitespace David Bremner 1 sibling, 1 reply; 6+ messages in thread From: david @ 2012-12-26 19:36 UTC (permalink / raw) To: notmuch; +Cc: David Bremner From: David Bremner <bremner@debian.org> This patch corrects several undesirable behaviours: 1) Empty files were not detected, leading to buffer read overrun. 2) An initial blank line cause restore to silently abort 3) Initial comment line caused format detection to fail --- notmuch-restore.c | 17 ++++++++++++----- test/dump-restore | 3 --- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 9ed9b51..c93f1ac 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -180,11 +180,6 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) argv[opt_index]); return 1; } - char *p; - - line_len = getline (&line, &line_size, input); - if (line_len == 0) - return 0; tag_ops = tag_op_list_create (ctx); if (tag_ops == NULL) { @@ -192,6 +187,18 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) return 1; } + do { + line_len = getline (&line, &line_size, input); + + /* empty input file not considered an error */ + if (line_len < 0) + return 0; + + } while ((line_len == 0) || + (line[0] == '#') || + (strspn (line, " \t\n") == strlen (line))); + + char *p; for (p = line; (input_format == DUMP_FORMAT_AUTO) && *p; p++) { if (*p == '(') input_format = DUMP_FORMAT_SUP; diff --git a/test/dump-restore b/test/dump-restore index c2ddb92..ae30cd1 100755 --- a/test/dump-restore +++ b/test/dump-restore @@ -146,13 +146,11 @@ cat <<EOF > comments-and-blanks EOF test_begin_subtest 'restoring empty file is not an error' -test_subtest_known_broken notmuch restore < /dev/null 2>OUTPUT.$test_count cp /dev/null EXPECTED test_expect_equal_file EXPECTED OUTPUT.$test_count test_begin_subtest 'file of comments and blank lines is not an error' -test_subtest_known_broken notmuch restore --input=comments-and-blanks ret_val=$? test_expect_equal "$ret_val" "0" @@ -172,7 +170,6 @@ echo "yun1vjwegii.fsf@aiko.keithp.com (another_tag)" \ >> leading-comments-blanks-sup test_begin_subtest 'detect format=sup with leading comments and blanks' -test_subtest_known_broken notmuch restore --input=leading-comments-blanks-sup notmuch search --output=tags id:yun1vjwegii.fsf@aiko.keithp.com > OUTPUT.$test_count echo "another_tag" > EXPECTED -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] notmuch-restore: handle empty input file, leading blank lines and comments. 2012-12-26 19:36 ` [PATCH 2/2] notmuch-restore: handle empty input file, leading blank lines and comments david @ 2013-01-05 22:11 ` Tomi Ollila 2013-01-06 14:04 ` [Patch v2] " david 0 siblings, 1 reply; 6+ messages in thread From: Tomi Ollila @ 2013-01-05 22:11 UTC (permalink / raw) To: david, notmuch; +Cc: David Bremner On Wed, Dec 26 2012, david@tethera.net wrote: > From: David Bremner <bremner@debian.org> > > This patch corrects several undesirable behaviours: > > 1) Empty files were not detected, leading to buffer read overrun. > > 2) An initial blank line cause restore to silently abort > > 3) Initial comment line caused format detection to fail > --- > notmuch-restore.c | 17 ++++++++++++----- > test/dump-restore | 3 --- > 2 files changed, 12 insertions(+), 8 deletions(-) > > diff --git a/notmuch-restore.c b/notmuch-restore.c > index 9ed9b51..c93f1ac 100644 > --- a/notmuch-restore.c > +++ b/notmuch-restore.c > @@ -180,11 +180,6 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) > argv[opt_index]); > return 1; > } > - char *p; > - > - line_len = getline (&line, &line_size, input); > - if (line_len == 0) > - return 0; > > tag_ops = tag_op_list_create (ctx); > if (tag_ops == NULL) { > @@ -192,6 +187,18 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) > return 1; > } > > + do { > + line_len = getline (&line, &line_size, input); > + > + /* empty input file not considered an error */ > + if (line_len < 0) > + return 0; > + > + } while ((line_len == 0) || > + (line[0] == '#') || > + (strspn (line, " \t\n") == strlen (line))); The strspn -- strlen doesn't take embedded \0:s in input line into consideration (and strlen() scans the full line again), (strspn (line, " \t\n") == line_len)); might better (if not, then line[strspn(line, " \t\n")] == '\0' would drop length scan) Otherwise LGTM. Tomi > + > + char *p; > for (p = line; (input_format == DUMP_FORMAT_AUTO) && *p; p++) { > if (*p == '(') > input_format = DUMP_FORMAT_SUP; > diff --git a/test/dump-restore b/test/dump-restore > index c2ddb92..ae30cd1 100755 > --- a/test/dump-restore > +++ b/test/dump-restore > @@ -146,13 +146,11 @@ cat <<EOF > comments-and-blanks > EOF > > test_begin_subtest 'restoring empty file is not an error' > -test_subtest_known_broken > notmuch restore < /dev/null 2>OUTPUT.$test_count > cp /dev/null EXPECTED > test_expect_equal_file EXPECTED OUTPUT.$test_count > > test_begin_subtest 'file of comments and blank lines is not an error' > -test_subtest_known_broken > notmuch restore --input=comments-and-blanks > ret_val=$? > test_expect_equal "$ret_val" "0" > @@ -172,7 +170,6 @@ echo "yun1vjwegii.fsf@aiko.keithp.com (another_tag)" \ > >> leading-comments-blanks-sup > > test_begin_subtest 'detect format=sup with leading comments and blanks' > -test_subtest_known_broken > notmuch restore --input=leading-comments-blanks-sup > notmuch search --output=tags id:yun1vjwegii.fsf@aiko.keithp.com > OUTPUT.$test_count > echo "another_tag" > EXPECTED > -- > 1.7.10.4 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Patch v2] notmuch-restore: handle empty input file, leading blank lines and comments. 2013-01-05 22:11 ` Tomi Ollila @ 2013-01-06 14:04 ` david 2013-01-06 15:28 ` Tomi Ollila 0 siblings, 1 reply; 6+ messages in thread From: david @ 2013-01-06 14:04 UTC (permalink / raw) To: notmuch; +Cc: David Bremner From: David Bremner <bremner@debian.org> This patch corrects several undesirable behaviours: 1) Empty files were not detected, leading to buffer read overrun. 2) An initial blank line cause restore to silently abort 3) Initial comment line caused format detection to fail --- notmuch-restore.c | 18 +++++++++++++----- test/dump-restore | 3 --- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index d43546d..f436989 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -181,11 +181,6 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) argv[opt_index]); return 1; } - char *p; - - line_len = getline (&line, &line_size, input); - if (line_len == 0) - return 0; tag_ops = tag_op_list_create (ctx); if (tag_ops == NULL) { @@ -193,6 +188,19 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) return 1; } + do { + line_len = getline (&line, &line_size, input); + + /* empty input file not considered an error */ + if (line_len < 0) + return 0; + + } while ((line_len == 0) || + (line[0] == '#') || + /* the cast is safe because we checked about for line_len < 0 */ + (strspn (line, " \t\n") == (unsigned)line_len)); + + char *p; for (p = line; (input_format == DUMP_FORMAT_AUTO) && *p; p++) { if (*p == '(') input_format = DUMP_FORMAT_SUP; diff --git a/test/dump-restore b/test/dump-restore index c2ddb92..ae30cd1 100755 --- a/test/dump-restore +++ b/test/dump-restore @@ -146,13 +146,11 @@ cat <<EOF > comments-and-blanks EOF test_begin_subtest 'restoring empty file is not an error' -test_subtest_known_broken notmuch restore < /dev/null 2>OUTPUT.$test_count cp /dev/null EXPECTED test_expect_equal_file EXPECTED OUTPUT.$test_count test_begin_subtest 'file of comments and blank lines is not an error' -test_subtest_known_broken notmuch restore --input=comments-and-blanks ret_val=$? test_expect_equal "$ret_val" "0" @@ -172,7 +170,6 @@ echo "yun1vjwegii.fsf@aiko.keithp.com (another_tag)" \ >> leading-comments-blanks-sup test_begin_subtest 'detect format=sup with leading comments and blanks' -test_subtest_known_broken notmuch restore --input=leading-comments-blanks-sup notmuch search --output=tags id:yun1vjwegii.fsf@aiko.keithp.com > OUTPUT.$test_count echo "another_tag" > EXPECTED -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Patch v2] notmuch-restore: handle empty input file, leading blank lines and comments. 2013-01-06 14:04 ` [Patch v2] " david @ 2013-01-06 15:28 ` Tomi Ollila 0 siblings, 0 replies; 6+ messages in thread From: Tomi Ollila @ 2013-01-06 15:28 UTC (permalink / raw) To: david, notmuch; +Cc: David Bremner On Sun, Jan 06 2013, david@tethera.net wrote: > From: David Bremner <bremner@debian.org> > > This patch corrects several undesirable behaviours: > > 1) Empty files were not detected, leading to buffer read overrun. > > 2) An initial blank line cause restore to silently abort > > 3) Initial comment line caused format detection to fail > --- LGTM. Tomi > notmuch-restore.c | 18 +++++++++++++----- > test/dump-restore | 3 --- > 2 files changed, 13 insertions(+), 8 deletions(-) > > diff --git a/notmuch-restore.c b/notmuch-restore.c > index d43546d..f436989 100644 > --- a/notmuch-restore.c > +++ b/notmuch-restore.c > @@ -181,11 +181,6 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) > argv[opt_index]); > return 1; > } > - char *p; > - > - line_len = getline (&line, &line_size, input); > - if (line_len == 0) > - return 0; > > tag_ops = tag_op_list_create (ctx); > if (tag_ops == NULL) { > @@ -193,6 +188,19 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) > return 1; > } > > + do { > + line_len = getline (&line, &line_size, input); > + > + /* empty input file not considered an error */ > + if (line_len < 0) > + return 0; > + > + } while ((line_len == 0) || > + (line[0] == '#') || > + /* the cast is safe because we checked about for line_len < 0 */ > + (strspn (line, " \t\n") == (unsigned)line_len)); > + > + char *p; > for (p = line; (input_format == DUMP_FORMAT_AUTO) && *p; p++) { > if (*p == '(') > input_format = DUMP_FORMAT_SUP; > diff --git a/test/dump-restore b/test/dump-restore > index c2ddb92..ae30cd1 100755 > --- a/test/dump-restore > +++ b/test/dump-restore > @@ -146,13 +146,11 @@ cat <<EOF > comments-and-blanks > EOF > > test_begin_subtest 'restoring empty file is not an error' > -test_subtest_known_broken > notmuch restore < /dev/null 2>OUTPUT.$test_count > cp /dev/null EXPECTED > test_expect_equal_file EXPECTED OUTPUT.$test_count > > test_begin_subtest 'file of comments and blank lines is not an error' > -test_subtest_known_broken > notmuch restore --input=comments-and-blanks > ret_val=$? > test_expect_equal "$ret_val" "0" > @@ -172,7 +170,6 @@ echo "yun1vjwegii.fsf@aiko.keithp.com (another_tag)" \ > >> leading-comments-blanks-sup > > test_begin_subtest 'detect format=sup with leading comments and blanks' > -test_subtest_known_broken > notmuch restore --input=leading-comments-blanks-sup > notmuch search --output=tags id:yun1vjwegii.fsf@aiko.keithp.com > OUTPUT.$test_count > echo "another_tag" > EXPECTED > -- > 1.7.10.4 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] test/dump-restore: new tests for empty files and leading comments/whitespace. 2012-12-26 19:36 [PATCH 1/2] test/dump-restore: new tests for empty files and leading comments/whitespace david 2012-12-26 19:36 ` [PATCH 2/2] notmuch-restore: handle empty input file, leading blank lines and comments david @ 2013-01-07 2:53 ` David Bremner 1 sibling, 0 replies; 6+ messages in thread From: David Bremner @ 2013-01-07 2:53 UTC (permalink / raw) To: notmuch david@tethera.net writes: > From: David Bremner <bremner@debian.org> > > Three of these are marked broken; the third is a regression test, > since it passes by virtue of batch-tag being the default input format. pushed, with v2 of 2/2. d ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-01-07 2:53 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-12-26 19:36 [PATCH 1/2] test/dump-restore: new tests for empty files and leading comments/whitespace david 2012-12-26 19:36 ` [PATCH 2/2] notmuch-restore: handle empty input file, leading blank lines and comments david 2013-01-05 22:11 ` Tomi Ollila 2013-01-06 14:04 ` [Patch v2] " david 2013-01-06 15:28 ` Tomi Ollila 2013-01-07 2:53 ` [PATCH 1/2] test/dump-restore: new tests for empty files and leading comments/whitespace David Bremner
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).