Return-Path: X-Original-To: notmuch@notmuchmail.org Delivered-To: notmuch@notmuchmail.org Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id 8C3A4431FAF for ; Sat, 5 Jan 2013 14:12:08 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: 0 X-Spam-Level: X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id U6QxXQAUoQkD for ; Sat, 5 Jan 2013 14:12:05 -0800 (PST) Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34]) by olra.theworths.org (Postfix) with ESMTP id D326E431FAE for ; Sat, 5 Jan 2013 14:12:04 -0800 (PST) Received: from guru.guru-group.fi (localhost [IPv6:::1]) by guru.guru-group.fi (Postfix) with ESMTP id EEA8710007D; Sun, 6 Jan 2013 00:11:56 +0200 (EET) From: Tomi Ollila To: david@tethera.net, notmuch@notmuchmail.org Subject: Re: [PATCH 2/2] notmuch-restore: handle empty input file, leading blank lines and comments. In-Reply-To: <1356550593-21683-2-git-send-email-david@tethera.net> References: <1356550593-21683-1-git-send-email-david@tethera.net> <1356550593-21683-2-git-send-email-david@tethera.net> User-Agent: Notmuch/0.14+236~ge6d0259 (http://notmuchmail.org) Emacs/24.2.1 (x86_64-unknown-linux-gnu) X-Face: HhBM'cA~ MIME-Version: 1.0 Content-Type: text/plain Cc: David Bremner X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 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: Sat, 05 Jan 2013 22:12:08 -0000 On Wed, Dec 26 2012, david@tethera.net wrote: > From: David Bremner > > 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 < 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