diff --git a/src/journal/test-journal.c b/src/journal/test-journal.c index cd295b2a7e5..b227b9be6cd 100644 --- a/src/journal/test-journal.c +++ b/src/journal/test-journal.c @@ -74,17 +74,17 @@ static void test_non_empty_one(void) { assert_se(le64toh(o->entry.seqnum) == 1); assert_se(journal_file_find_data_object(f->file, test, strlen(test), &d, NULL) == 1); - assert_se(journal_file_next_entry_for_data(f->file, d, DIRECTION_DOWN, &o, NULL) == 1); + assert_se(journal_file_move_to_entry_for_data(f->file, d, DIRECTION_DOWN, &o, NULL) == 1); assert_se(le64toh(o->entry.seqnum) == 1); - assert_se(journal_file_next_entry_for_data(f->file, d, DIRECTION_UP, &o, NULL) == 1); + assert_se(journal_file_move_to_entry_for_data(f->file, d, DIRECTION_UP, &o, NULL) == 1); assert_se(le64toh(o->entry.seqnum) == 3); assert_se(journal_file_find_data_object(f->file, test2, strlen(test2), &d, NULL) == 1); - assert_se(journal_file_next_entry_for_data(f->file, d, DIRECTION_UP, &o, NULL) == 1); + assert_se(journal_file_move_to_entry_for_data(f->file, d, DIRECTION_UP, &o, NULL) == 1); assert_se(le64toh(o->entry.seqnum) == 2); - assert_se(journal_file_next_entry_for_data(f->file, d, DIRECTION_DOWN, &o, NULL) == 1); + assert_se(journal_file_move_to_entry_for_data(f->file, d, DIRECTION_DOWN, &o, NULL) == 1); assert_se(le64toh(o->entry.seqnum) == 2); assert_se(journal_file_find_data_object(f->file, "quux", 4, &d, NULL) == 0); diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 5d3f85e4cc3..1b80afe74dc 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -2628,11 +2628,12 @@ int journal_file_append_entry( } typedef struct ChainCacheItem { - uint64_t first; /* the array at the beginning of the chain */ - uint64_t array; /* the cached array */ - uint64_t begin; /* the first item in the cached array */ - uint64_t total; /* the total number of items in all arrays before this one in the chain */ - uint64_t last_index; /* the last index we looked at, to optimize locality when bisecting */ + uint64_t first; /* The offset of the entry array object at the beginning of the chain, + * i.e., le64toh(f->header->entry_array_offset), or le64toh(o->data.entry_offset). */ + uint64_t array; /* The offset of the cached entry array object. */ + uint64_t begin; /* The offset of the first item in the cached array. */ + uint64_t total; /* The total number of items in all arrays before the cached one in the chain. */ + uint64_t last_index; /* The last index we looked at in the cached array, to optimize locality when bisecting. */ } ChainCacheItem; static void chain_cache_put( @@ -2698,60 +2699,61 @@ static int bump_array_index(uint64_t *i, direction_t direction, uint64_t n) { static int bump_entry_array( JournalFile *f, - Object *o, - uint64_t offset, - uint64_t first, + Object *o, /* the current entry array object. */ + uint64_t offset, /* the offset of the entry array object. */ + uint64_t first, /* The offset of the first entry array object in the chain. */ direction_t direction, uint64_t *ret) { - uint64_t p, q = 0; int r; assert(f); - assert(offset); assert(ret); if (direction == DIRECTION_DOWN) { assert(o); + assert(o->object.type == OBJECT_ENTRY_ARRAY); + *ret = le64toh(o->entry_array.next_entry_array_offset); - return 0; + } else { + + /* Entry array chains are a singly linked list, so to find the previous array in the chain, we have + * to start iterating from the top. */ + + assert(offset > 0); + + uint64_t p = first, q = 0; + while (p > 0 && p != offset) { + r = journal_file_move_to_object(f, OBJECT_ENTRY_ARRAY, p, &o); + if (r < 0) + return r; + + q = p; + p = le64toh(o->entry_array.next_entry_array_offset); + } + + /* If we can't find the previous entry array in the entry array chain, we're likely dealing with a + * corrupted journal file. */ + if (p == 0) + return -EBADMSG; + + *ret = q; } - /* Entry array chains are a singly linked list, so to find the previous array in the chain, we have - * to start iterating from the top. */ - - p = first; - - while (p > 0 && p != offset) { - r = journal_file_move_to_object(f, OBJECT_ENTRY_ARRAY, p, &o); - if (r < 0) - return r; - - q = p; - p = le64toh(o->entry_array.next_entry_array_offset); - } - - /* If we can't find the previous entry array in the entry array chain, we're likely dealing with a - * corrupted journal file. */ - if (p == 0) - return -EBADMSG; - - *ret = q; - - return 0; + return *ret > 0; } static int generic_array_get( JournalFile *f, - uint64_t first, - uint64_t i, + uint64_t first, /* The offset of the first entry array object in the chain. */ + uint64_t i, /* The index of the target object counted from the beginning of the entry array chain. */ direction_t direction, - Object **ret_object, - uint64_t *ret_offset) { + Object **ret_object, /* The found object. */ + uint64_t *ret_offset) { /* The offset of the found object. */ uint64_t a, t = 0, k; ChainCacheItem *ci; - Object *o; + Object *o = NULL; int r; assert(f); @@ -2780,21 +2782,20 @@ static int generic_array_get( /* If there's corruption and we're going upwards, move back to the previous entry * array and start iterating entries from there. */ - r = bump_entry_array(f, NULL, a, first, DIRECTION_UP, &a); - if (r < 0) - return r; - i = UINT64_MAX; - break; } if (r < 0) return r; k = journal_file_entry_array_n_items(f, o); + if (k == 0) + return 0; + if (i < k) break; + /* The index is larger than the number of elements in the array. Let's move to the next array. */ i -= k; t += k; a = le64toh(o->entry_array.next_entry_array_offset); @@ -2804,9 +2805,11 @@ static int generic_array_get( * direction). */ while (a > 0) { - /* In the first iteration of the while loop, we reuse i, k and o from the previous while - * loop. */ if (i == UINT64_MAX) { + r = bump_entry_array(f, o, a, first, direction, &a); + if (r <= 0) + return r; + r = journal_file_move_to_object(f, OBJECT_ENTRY_ARRAY, a, &o); if (r < 0) return r; @@ -2815,7 +2818,16 @@ static int generic_array_get( if (k == 0) break; - i = direction == DIRECTION_DOWN ? 0 : k - 1; + if (direction == DIRECTION_DOWN) + i = 0; + else { + /* We moved to the previous array. The total must be decreased. */ + if (t < k) + return -EBADMSG; /* chain cache is broken ? */ + + i = k - 1; + t -= k; + } } do { @@ -2840,54 +2852,20 @@ static int generic_array_get( * disk properly, let's see if the next one might work for us instead. */ log_debug_errno(r, "Entry item %" PRIu64 " is bad, skipping over it.", i); - r = journal_file_move_to_object(f, OBJECT_ENTRY_ARRAY, a, &o); - if (r < 0) - return r; - } while (bump_array_index(&i, direction, k) > 0); - r = bump_entry_array(f, o, a, first, direction, &a); - if (r < 0) - return r; + /* All entries tried in the above do-while loop are broken. Let's move to the next (or previous) array. */ + + if (direction == DIRECTION_DOWN) + /* We are going to the next array, the total must be incremented. */ + t += k; - t += k; i = UINT64_MAX; } return 0; } -static int generic_array_get_plus_one( - JournalFile *f, - uint64_t extra, - uint64_t first, - uint64_t i, - direction_t direction, - Object **ret_object, - uint64_t *ret_offset) { - - int r; - - assert(f); - - /* FIXME: fix return value assignment on success. */ - - if (i == 0) { - r = journal_file_move_to_object(f, OBJECT_ENTRY, extra, ret_object); - if (IN_SET(r, -EADDRNOTAVAIL, -EBADMSG)) - return generic_array_get(f, first, 0, direction, ret_object, ret_offset); - if (r < 0) - return r; - - if (ret_offset) - *ret_offset = extra; - - return 1; - } - - return generic_array_get(f, first, i - 1, direction, ret_object, ret_offset); -} - enum { TEST_FOUND, TEST_LEFT, @@ -3461,39 +3439,67 @@ int journal_file_next_entry( return 1; } -int journal_file_next_entry_for_data( +int journal_file_move_to_entry_for_data( JournalFile *f, Object *d, direction_t direction, Object **ret_object, uint64_t *ret_offset) { - uint64_t i, n, ofs; - int r; + uint64_t extra, first, n; + int r = 0; assert(f); assert(d); assert(d->object.type == OBJECT_DATA); + assert(IN_SET(direction, DIRECTION_DOWN, DIRECTION_UP)); /* FIXME: fix return value assignment. */ - n = le64toh(READ_NOW(d->data.n_entries)); + /* This returns the first (when the direction is down, otherwise the last) entry linked to the + * specified data object. */ + + n = le64toh(d->data.n_entries); if (n <= 0) - return n; + return 0; + n--; /* n_entries is the number of entries linked to the data object, including the 'extra' entry. */ - i = direction == DIRECTION_DOWN ? 0 : n - 1; + extra = le64toh(d->data.entry_offset); + first = le64toh(d->data.entry_array_offset); - r = generic_array_get_plus_one(f, - le64toh(d->data.entry_offset), - le64toh(d->data.entry_array_offset), - i, - direction, - ret_object, &ofs); - if (r <= 0) - return r; + if (direction == DIRECTION_DOWN && extra > 0) { + /* When we are going downwards, first try to read the extra entry. */ + r = journal_file_move_to_object(f, OBJECT_ENTRY, extra, ret_object); + if (r >= 0) + goto use_extra; + if (!IN_SET(r, -EADDRNOTAVAIL, -EBADMSG)) + return r; + } + if (n > 0) { + /* DIRECTION_DOWN : The extra entry is broken, falling back to the entries in the array. + * DIRECTION_UP : Try to find a valid entry in the array from the tail. */ + r = generic_array_get(f, + first, + direction == DIRECTION_DOWN ? 0 : n - 1, + direction, + ret_object, ret_offset); + if (!IN_SET(r, 0, -EADDRNOTAVAIL, -EBADMSG)) + return r; /* found or critical error. */ + } + + if (direction == DIRECTION_UP && extra > 0) { + /* No valid entry exists in the chained array, falling back to the extra entry. */ + r = journal_file_move_to_object(f, OBJECT_ENTRY, extra, ret_object); + if (r >= 0) + goto use_extra; + } + + return r; + +use_extra: if (ret_offset) - *ret_offset = ofs; + *ret_offset = extra; return 1; } @@ -4455,12 +4461,7 @@ int journal_file_get_cutoff_monotonic_usec(JournalFile *f, sd_id128_t boot_id, u if (r < 0) return r; - r = generic_array_get_plus_one(f, - le64toh(o->data.entry_offset), - le64toh(o->data.entry_array_offset), - le64toh(o->data.n_entries) - 1, - DIRECTION_UP, - &o, NULL); + r = journal_file_move_to_entry_for_data(f, o, DIRECTION_UP, &o, NULL); if (r <= 0) return r; diff --git a/src/libsystemd/sd-journal/journal-file.h b/src/libsystemd/sd-journal/journal-file.h index b018fd8788e..6c46dff4c45 100644 --- a/src/libsystemd/sd-journal/journal-file.h +++ b/src/libsystemd/sd-journal/journal-file.h @@ -286,13 +286,13 @@ void journal_file_reset_location(JournalFile *f); void journal_file_save_location(JournalFile *f, Object *o, uint64_t offset); int journal_file_next_entry(JournalFile *f, uint64_t p, direction_t direction, Object **ret_object, uint64_t *ret_offset); -int journal_file_next_entry_for_data(JournalFile *f, Object *d, direction_t direction, Object **ret_object, uint64_t *ret_offset); - int journal_file_move_to_entry_by_offset(JournalFile *f, uint64_t p, direction_t direction, Object **ret_object, uint64_t *ret_offset); int journal_file_move_to_entry_by_seqnum(JournalFile *f, uint64_t seqnum, direction_t direction, Object **ret_object, uint64_t *ret_offset); int journal_file_move_to_entry_by_realtime(JournalFile *f, uint64_t realtime, direction_t direction, Object **ret_object, uint64_t *ret_offset); int journal_file_move_to_entry_by_monotonic(JournalFile *f, sd_id128_t boot_id, uint64_t monotonic, direction_t direction, Object **ret_object, uint64_t *ret_offset); +int journal_file_move_to_entry_for_data(JournalFile *f, Object *d, direction_t direction, Object **ret_object, uint64_t *ret_offset); + int journal_file_move_to_entry_by_offset_for_data(JournalFile *f, Object *d, uint64_t p, direction_t direction, Object **ret_object, uint64_t *ret_offset); int journal_file_move_to_entry_by_seqnum_for_data(JournalFile *f, Object *d, uint64_t seqnum, direction_t direction, Object **ret_object, uint64_t *ret_offset); int journal_file_move_to_entry_by_realtime_for_data(JournalFile *f, Object *d, uint64_t realtime, direction_t direction, Object **ret_object, uint64_t *ret_offset); diff --git a/src/libsystemd/sd-journal/sd-journal.c b/src/libsystemd/sd-journal/sd-journal.c index 1b9dcbdecb3..3b8ca21725c 100644 --- a/src/libsystemd/sd-journal/sd-journal.c +++ b/src/libsystemd/sd-journal/sd-journal.c @@ -660,9 +660,9 @@ static int find_location_for_match( /* FIXME: missing: find by monotonic */ if (j->current_location.type == LOCATION_HEAD) - return direction == DIRECTION_DOWN ? journal_file_next_entry_for_data(f, d, DIRECTION_DOWN, ret, offset) : 0; + return direction == DIRECTION_DOWN ? journal_file_move_to_entry_for_data(f, d, DIRECTION_DOWN, ret, offset) : 0; if (j->current_location.type == LOCATION_TAIL) - return direction == DIRECTION_UP ? journal_file_next_entry_for_data(f, d, DIRECTION_UP, ret, offset) : 0; + return direction == DIRECTION_UP ? journal_file_move_to_entry_for_data(f, d, DIRECTION_UP, ret, offset) : 0; if (j->current_location.seqnum_set && sd_id128_equal(j->current_location.seqnum_id, f->header->seqnum_id)) return journal_file_move_to_entry_by_seqnum_for_data(f, d, j->current_location.seqnum, direction, ret, offset); if (j->current_location.monotonic_set) { @@ -678,7 +678,7 @@ static int find_location_for_match( if (j->current_location.realtime_set) return journal_file_move_to_entry_by_realtime_for_data(f, d, j->current_location.realtime, direction, ret, offset); - return journal_file_next_entry_for_data(f, d, direction, ret, offset); + return journal_file_move_to_entry_for_data(f, d, direction, ret, offset); } else if (m->type == MATCH_OR_TERM) { uint64_t np = 0;