From: Yu Watanabe <watanabe.yu+github@gmail.com>
Date: Sun, 21 May 2023 16:56:08 +0000 (+0900)
Subject: test-journal-interleaving: extend tests to clarify the issue in sd_journal_next(... 
X-Git-Tag: v254-rc1~426^2~2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=43e460a5842ee42b9f62fbd690e48e823364b03e;p=thirdparty%2Fsystemd.git

test-journal-interleaving: extend tests to clarify the issue in sd_journal_next() or friends

This illustrates bug in sd_journal_next() or friends;
calling sd_jounral_next() followed by sd_journal_seek_tail() makes the
location saved in sd-journal something corrupted, and subsequent
sd_journal_previous() or friends may fail or provides unexpected result.
---

diff --git a/src/journal/test-journal-interleaving.c b/src/journal/test-journal-interleaving.c
index 7fec6d9eea2..08b793c6182 100644
--- a/src/journal/test-journal-interleaving.c
+++ b/src/journal/test-journal-interleaving.c
@@ -78,9 +78,9 @@ static void test_check_number(sd_journal *j, int n) {
 
         assert_ret(sd_journal_get_data(j, "NUMBER", &d, &l));
         assert_se(k = strndup(d, l));
-        printf("%s\n", k);
+        printf("%s (expected=%i)\n", k, n);
 
-        assert_se(safe_atoi(k + 7, &x) >= 0);
+        assert_se(safe_atoi(k + STRLEN("NUMBER="), &x) >= 0);
         assert_se(n == x);
 }
 
@@ -113,27 +113,43 @@ static void test_check_numbers_up(sd_journal *j, int count) {
 }
 
 static void setup_sequential(void) {
-        ManagedJournalFile *one, *two;
-        one = test_open("one.journal");
-        two = test_open("two.journal");
-        append_number(one, 1, NULL);
-        append_number(one, 2, NULL);
-        append_number(two, 3, NULL);
-        append_number(two, 4, NULL);
-        test_close(one);
-        test_close(two);
+        ManagedJournalFile *f1, *f2, *f3;
+
+        f1 = test_open("one.journal");
+        f2 = test_open("two.journal");
+        f3 = test_open("three.journal");
+        append_number(f1, 1, NULL);
+        append_number(f1, 2, NULL);
+        append_number(f1, 3, NULL);
+        append_number(f2, 4, NULL);
+        append_number(f2, 5, NULL);
+        append_number(f2, 6, NULL);
+        append_number(f3, 7, NULL);
+        append_number(f3, 8, NULL);
+        append_number(f3, 9, NULL);
+        test_close(f1);
+        test_close(f2);
+        test_close(f3);
 }
 
 static void setup_interleaved(void) {
-        ManagedJournalFile *one, *two;
-        one = test_open("one.journal");
-        two = test_open("two.journal");
-        append_number(one, 1, NULL);
-        append_number(two, 2, NULL);
-        append_number(one, 3, NULL);
-        append_number(two, 4, NULL);
-        test_close(one);
-        test_close(two);
+        ManagedJournalFile *f1, *f2, *f3;
+
+        f1 = test_open("one.journal");
+        f2 = test_open("two.journal");
+        f3 = test_open("three.journal");
+        append_number(f1, 1, NULL);
+        append_number(f2, 2, NULL);
+        append_number(f3, 3, NULL);
+        append_number(f1, 4, NULL);
+        append_number(f2, 5, NULL);
+        append_number(f3, 6, NULL);
+        append_number(f1, 7, NULL);
+        append_number(f2, 8, NULL);
+        append_number(f3, 9, NULL);
+        test_close(f1);
+        test_close(f2);
+        test_close(f3);
 }
 
 static void mkdtemp_chdir_chattr(char *path) {
@@ -154,38 +170,140 @@ static void test_skip_one(void (*setup)(void)) {
 
         setup();
 
-        /* Seek to head, iterate down.
-         */
+        /* Seek to head, iterate down. */
         assert_ret(sd_journal_open_directory(&j, t, 0));
         assert_ret(sd_journal_seek_head(j));
-        assert_ret(sd_journal_next(j));
-        test_check_numbers_down(j, 4);
+        assert_ret(sd_journal_next(j));     /* pointing the first entry */
+        test_check_numbers_down(j, 9);
+        sd_journal_close(j);
+
+        /* Seek to head, iterate down. */
+        assert_ret(sd_journal_open_directory(&j, t, 0));
+        assert_ret(sd_journal_seek_head(j));
+        assert_ret(sd_journal_next(j));     /* pointing the first entry */
+        assert_ret(sd_journal_previous(j)); /* no-op */
+        test_check_numbers_down(j, 9);
+        sd_journal_close(j);
+
+        /* Seek to head, move to previous, then iterate down. */
+        assert_ret(sd_journal_open_directory(&j, t, 0));
+        assert_ret(sd_journal_seek_head(j));
+        /* FIXME: calling sd_journal_previous() thrice (or >= number of journal files) here works as
+         * mostly equivalent to calling sd_journal_next(). */
+        assert_ret(sd_journal_previous(j));
+        assert_ret(sd_journal_previous(j));
+        assert_ret(sd_journal_previous(j)); /* pointing the first entry */
+        test_check_numbers_down(j, 9);
+        sd_journal_close(j);
+
+        /* Seek to head, walk several steps, then iterate down. */
+        assert_ret(sd_journal_open_directory(&j, t, 0));
+        assert_ret(sd_journal_seek_head(j));
+        assert_ret(sd_journal_previous(j));
+        assert_ret(sd_journal_previous(j));
+        assert_ret(sd_journal_previous(j)); /* pointing the first entry */
+        assert_ret(sd_journal_next(j));     /* pointing the second entry */
+        /* FIXME: as calling sd_journal_previous() thrice works equivalent to sd_journal_next(),
+         * we need to call sd_journal_previous() here. */
+        assert_ret(sd_journal_previous(j)); /* pointing the first entry */
+        assert_ret(sd_journal_previous(j)); /* no-op */
+        test_check_numbers_down(j, 9);
         sd_journal_close(j);
 
-        /* Seek to tail, iterate up.
-         */
+        /* Seek to tail, iterate up. */
         assert_ret(sd_journal_open_directory(&j, t, 0));
         assert_ret(sd_journal_seek_tail(j));
         assert_ret(sd_journal_previous(j));
-        test_check_numbers_up(j, 4);
+        test_check_numbers_up(j, 9);
         sd_journal_close(j);
 
-        /* Seek to tail, skip to head, iterate down.
-         */
+        /* Seek to tail, move to next, then iterate up. */
         assert_ret(sd_journal_open_directory(&j, t, 0));
         assert_ret(sd_journal_seek_tail(j));
+        /* FIXME: calling sd_journal_next() thrice (or >= number of journal files) here works as mostly
+         * equivalent to calling sd_journal_previous(). */
+        assert_ret(sd_journal_next(j));
+        assert_ret(sd_journal_next(j));
+        assert_ret(sd_journal_next(j)); /* pointing the last entry */
+        test_check_numbers_up(j, 9);
+        sd_journal_close(j);
+
+        /* Seek to tail, walk several steps, then iterate up. */
+        assert_ret(sd_journal_open_directory(&j, t, 0));
+        assert_ret(sd_journal_seek_tail(j));
+        assert_ret(sd_journal_next(j));
+        assert_ret(sd_journal_next(j));
+        assert_ret(sd_journal_next(j));     /* pointing the last entry */
+        assert_ret(sd_journal_previous(j)); /* pointing the previous of the last entry. */
+        assert_ret(sd_journal_next(j));     /* pointing the last entry */
+        assert_ret(sd_journal_next(j));     /* no-op */
+        test_check_numbers_up(j, 9);
+        sd_journal_close(j);
+
+        /* Seek to tail, skip to head, iterate down. */
+        assert_ret(sd_journal_open_directory(&j, t, 0));
+        assert_ret(sd_journal_seek_tail(j));
+        assert_ret(r = sd_journal_previous_skip(j, 9));
+        assert_se(r == 9);
+        test_check_numbers_down(j, 9);
+        sd_journal_close(j);
+
+        /* Seek to tail, skip to head in a more complext way, then iterate down. */
+        assert_ret(sd_journal_open_directory(&j, t, 0));
+        assert_ret(sd_journal_seek_tail(j));
+        // FIXME: sd_journal_next() cannot be called here.
+        //assert_ret(sd_journal_next(j));
         assert_ret(r = sd_journal_previous_skip(j, 4));
         assert_se(r == 4);
-        test_check_numbers_down(j, 4);
+        assert_ret(r = sd_journal_previous_skip(j, 5));
+        assert_se(r == 5);
+        assert_ret(sd_journal_previous(j)); /* no-op */
+        assert_ret(r = sd_journal_previous_skip(j, 5));
+        assert_se(r == 0);
+        assert_ret(sd_journal_next(j));
+        assert_ret(r = sd_journal_previous_skip(j, 5));
+        assert_se(r == 1);
+        assert_ret(sd_journal_next(j));
+        assert_ret(sd_journal_next(j));
+        assert_ret(sd_journal_previous(j));
+        assert_ret(sd_journal_next(j));
+        assert_ret(sd_journal_next(j));
+        assert_ret(r = sd_journal_previous_skip(j, 5));
+        assert_se(r == 3);
+        test_check_numbers_down(j, 9);
+        sd_journal_close(j);
+
+        /* Seek to head, skip to tail, iterate up. */
+        assert_ret(sd_journal_open_directory(&j, t, 0));
+        assert_ret(sd_journal_seek_head(j));
+        assert_ret(r = sd_journal_next_skip(j, 9));
+        assert_se(r == 9);
+        test_check_numbers_up(j, 9);
         sd_journal_close(j);
 
-        /* Seek to head, skip to tail, iterate up.
-         */
+        /* Seek to head, skip to tail in a more complex way, then iterate up. */
         assert_ret(sd_journal_open_directory(&j, t, 0));
         assert_ret(sd_journal_seek_head(j));
+        // FIXME: sd_journal_previous() cannot be called here.
+        //assert_ret(sd_journal_previous(j));
         assert_ret(r = sd_journal_next_skip(j, 4));
         assert_se(r == 4);
-        test_check_numbers_up(j, 4);
+        assert_ret(r = sd_journal_next_skip(j, 5));
+        assert_se(r == 5);
+        assert_ret(sd_journal_next(j));     /* no-op */
+        assert_ret(r = sd_journal_next_skip(j, 5));
+        assert_se(r == 0);
+        assert_ret(sd_journal_previous(j));
+        assert_ret(r = sd_journal_next_skip(j, 5));
+        assert_se(r == 1);
+        assert_ret(sd_journal_previous(j));
+        assert_ret(sd_journal_previous(j));
+        assert_ret(sd_journal_next(j));
+        assert_ret(sd_journal_previous(j));
+        assert_ret(sd_journal_previous(j));
+        assert_ret(r = sd_journal_next_skip(j, 5));
+        assert_se(r == 3);
+        test_check_numbers_up(j, 9);
         sd_journal_close(j);
 
         log_info("Done...");