diff --git a/src/shared/file_list.c b/src/shared/file_list.c index b01a114..3297a87 100644 --- a/src/shared/file_list.c +++ b/src/shared/file_list.c @@ -104,30 +104,20 @@ static int normalize_entry(const char* raw, size_t len, bool strip_line_endings, return result; } -/* Build the membership index: each non-empty entry plus every ancestor - directory prefix of it. The entry flag lets file_list_affects tell an exact - listed path from an ancestor of a listed path. An empty entry (the source - root) short-circuits every query, so it is recorded as whole_tree. */ +/* Build the membership index over the exact entries only. `file_list_affects` + combines the exact/descendant lookups with a walk of the query's own ancestor + prefixes, so no ancestor prefix is ever materialized as a copy and the index + stays O(entry count) memory regardless of path depth. An empty entry (the + source root) sets whole_tree and short-circuits every query. */ static bool file_list_index_build(FileListSet* set, char* err, size_t err_size) { - if (!str_hash_set_init(&set->node_index, (size_t)set->count * 2 + 1)) { + if (!path_index_build(&set->index, (const char* const*)set->entries, (size_t)set->count)) { snprintf(err, err_size, "memory allocation failed"); return false; } for (int i = 0; i < set->count; i++) { - const char* entry = set->entries[i]; - if (entry[0] == '\0') { + if (set->entries[i][0] == '\0') { set->whole_tree = true; - continue; - } - if (!str_hash_set_insert_ref(&set->node_index, entry, true)) { - snprintf(err, err_size, "memory allocation failed"); - return false; - } - for (const char* slash = entry; (slash = strchr(slash, '/')) != NULL; slash++) { - if (!str_hash_set_insert_copy_n(&set->node_index, entry, (size_t)(slash - entry), false)) { - snprintf(err, err_size, "memory allocation failed"); - return false; - } + break; } } return true; @@ -193,10 +183,10 @@ FileListSet* file_list_load(const char* path, bool null_separated, char* err, si void file_list_destroy(FileListSet* set) { if (!set) return; + path_index_free(&set->index); for (int i = 0; i < set->count; i++) free(set->entries[i]); free(set->entries); - str_hash_set_free(&set->node_index); free(set); } @@ -207,13 +197,12 @@ bool file_list_affects(const FileListSet* set, const char* rel) { return false; if (set->whole_tree) return true; /* whole tree listed */ - /* A node hit means `rel` is a listed entry, or an ancestor directory of one - (rel lives on the path to some listed entry). */ - if (str_hash_set_lookup(&set->node_index, rel, NULL)) + /* An exact entry match means `rel` itself is listed. */ + if (path_index_contains(&set->index, rel)) return true; - /* Otherwise `rel` is affected only when a listed entry is an ancestor of it; - walk rel's directory prefixes (which preserve path-boundary semantics) and - test each for an exact entry. */ + /* Otherwise `rel` is affected when a listed entry is an ancestor directory of + it; walk rel's own directory prefixes (which preserve path-boundary + semantics) and test each for an exact entry. No prefixes are stored. */ size_t len = strlen(rel); while (len > 0) { const char* slash = NULL; @@ -226,9 +215,10 @@ bool file_list_affects(const FileListSet* set, const char* rel) { if (!slash) break; len = (size_t)(slash - rel); - bool is_entry = false; - if (str_hash_set_lookup_n(&set->node_index, rel, len, &is_entry) && is_entry) + if (path_index_contains_n(&set->index, rel, len)) return true; } - return false; + /* Finally `rel` is affected when it is an ancestor directory of a listed + entry (binary search for the first entry at or after `rel` + '/'). */ + return path_index_has_descendant(&set->index, rel); } diff --git a/src/shared/file_list.h b/src/shared/file_list.h index 53c1a79..b18a8ee 100644 --- a/src/shared/file_list.h +++ b/src/shared/file_list.h @@ -14,14 +14,16 @@ * rejected at parse time. The set is immutable and shared read-only across * scanner worker threads. * - * Membership is answered from `node_index`, built once at load time: it holds - * every entry plus every ancestor directory prefix of an entry, with the entry - * flag distinguishing an exact listed path from a mere ancestor. A lookup is - * O(path length) instead of O(entry count). */ + * Membership is answered from `index`, built once at load time over the exact + * entries only: `index.exact` matches a listed path, the sorted view detects an + * ancestor directory of a listed entry, and `rel`'s own directory prefixes are + * matched against the exact set while descending. No ancestor prefix is stored + * as a separate string, so the index is O(entry count) memory however deep the + * paths are, and each query is O(path length) comparisons. */ typedef struct { char** entries; /* normalized rel paths; "" means the whole tree */ int count; - StrHashSet node_index; + PathIndex index; bool whole_tree; /* an entry of "" lists the source root */ } FileListSet; diff --git a/src/shared/utils.c b/src/shared/utils.c index e15d55b..17b1d50 100644 --- a/src/shared/utils.c +++ b/src/shared/utils.c @@ -103,26 +103,20 @@ static size_t str_hash_set_hash(const char* key, size_t len) { return (size_t)XXH64(key, len, 0); } -/* Store an already-allocated key. Returns 1 when a new slot was filled and 0 - * for a duplicate (the caller keeps ownership of `key` when owned is true). */ -static int str_hash_set_put(StrHashSet* set, const char* key, size_t len, bool owned, - bool is_entry) { +/* Store a borrowed key. Returns 1 when a new slot was filled and 0 for a + * duplicate. */ +static int str_hash_set_put(StrHashSet* set, const char* key, size_t len) { size_t mask = set->capacity - 1; size_t index = str_hash_set_hash(key, len) & mask; while (true) { StrHashSetSlot* slot = &set->slots[index]; if (!slot->key) { slot->key = key; - slot->owned = owned; - slot->is_entry = is_entry; set->size++; return 1; } - if (strlen(slot->key) == len && memcmp(slot->key, key, len) == 0) { - if (is_entry) - slot->is_entry = true; + if (strlen(slot->key) == len && memcmp(slot->key, key, len) == 0) return 0; - } index = (index + 1) & mask; } } @@ -138,8 +132,7 @@ static bool str_hash_set_resize(StrHashSet* set, size_t new_capacity) { set->size = 0; for (size_t i = 0; i < old_capacity; i++) { if (old_slots[i].key) - (void)str_hash_set_put(set, old_slots[i].key, strlen(old_slots[i].key), old_slots[i].owned, - old_slots[i].is_entry); + (void)str_hash_set_put(set, old_slots[i].key, strlen(old_slots[i].key)); } free(old_slots); return true; @@ -159,7 +152,7 @@ bool str_hash_set_init(StrHashSet* set, size_t hint) { set->capacity = 0; set->size = 0; size_t capacity = STR_HASH_SET_MIN_CAPACITY; - while (capacity < (hint + 1) * 2) + while (capacity < (hint + 1) * 2 && capacity <= SIZE_MAX / 2) capacity *= 2; set->slots = calloc(capacity, sizeof(StrHashSetSlot)); if (!set->slots) @@ -171,46 +164,18 @@ bool str_hash_set_init(StrHashSet* set, size_t hint) { void str_hash_set_free(StrHashSet* set) { if (!set) return; - for (size_t i = 0; i < set->capacity; i++) { - if (set->slots[i].key && set->slots[i].owned) - free((void*)set->slots[i].key); - } free(set->slots); set->slots = NULL; set->capacity = 0; set->size = 0; } -bool str_hash_set_insert_ref(StrHashSet* set, const char* key, bool is_entry) { +bool str_hash_set_insert_ref(StrHashSet* set, const char* key) { if (!set || !key) return false; if (!str_hash_set_grow(set)) return false; - return str_hash_set_put(set, key, strlen(key), false, is_entry) >= 0; -} - -bool str_hash_set_insert_copy_n(StrHashSet* set, const char* key, size_t len, bool is_entry) { - if (!set || !key) - return false; - bool present = false; - if (str_hash_set_lookup_n(set, key, len, &present)) { - if (is_entry) - (void)str_hash_set_put(set, key, len, false, true); /* upgrade in place */ - return true; - } - if (!str_hash_set_grow(set)) - return false; - char* copy = malloc(len + 1); - if (!copy) - return false; - memcpy(copy, key, len); - copy[len] = '\0'; - int result = str_hash_set_put(set, copy, len, true, is_entry); - if (result <= 0) { - free(copy); - return result == 0; - } - return true; + return str_hash_set_put(set, key, strlen(key)) >= 0; } static const StrHashSetSlot* str_hash_set_find_n(const StrHashSet* set, const char* key, @@ -229,19 +194,140 @@ static const StrHashSetSlot* str_hash_set_find_n(const StrHashSet* set, const ch } } -bool str_hash_set_lookup_n(const StrHashSet* set, const char* key, size_t len, bool* is_entry) { - const StrHashSetSlot* slot = str_hash_set_find_n(set, key, len); - if (!slot) +bool str_hash_set_lookup_n(const StrHashSet* set, const char* key, size_t len) { + return str_hash_set_find_n(set, key, len) != NULL; +} + +bool str_hash_set_lookup(const StrHashSet* set, const char* key) { + if (!key) return false; - if (is_entry) - *is_entry = slot->is_entry; + return str_hash_set_lookup_n(set, key, strlen(key)); +} + +static int str_sorted_array_compare(const void* left, const void* right) { + const char* const* left_key = left; + const char* const* right_key = right; + return strcmp(*left_key, *right_key); +} + +bool str_sorted_array_build(StrSortedArray* array, const char* const* items, size_t count) { + if (!array) + return false; + array->items = NULL; + array->count = 0; + if (count == 0) + return true; + if (!items || count > SIZE_MAX / sizeof(const char*)) + return false; + const char** sorted = malloc(count * sizeof(*sorted)); + if (!sorted) + return false; + for (size_t i = 0; i < count; i++) + sorted[i] = items[i]; + qsort(sorted, count, sizeof(*sorted), str_sorted_array_compare); + array->items = sorted; + array->count = count; return true; } -bool str_hash_set_lookup(const StrHashSet* set, const char* key, bool* is_entry) { - if (!key) +void str_sorted_array_free(StrSortedArray* array) { + if (!array) + return; + free(array->items); + array->items = NULL; + array->count = 0; +} + +bool str_sorted_array_contains(const StrSortedArray* array, const char* key) { + if (!array || !key || array->count == 0) return false; - return str_hash_set_lookup_n(set, key, strlen(key), is_entry); + size_t lo = 0; + size_t hi = array->count; + while (lo < hi) { + size_t mid = lo + (hi - lo) / 2; + int cmp = strcmp(array->items[mid], key); + if (cmp < 0) + lo = mid + 1; + else if (cmp > 0) + hi = mid; + else + return true; + } + return false; +} + +/* Compare `entry` against the virtual key `key` + '/' without allocating the + * concatenation. Returns <0, 0 or >0 as `entry` sorts before, equal to, or + * after that virtual key. */ +static int str_sorted_array_compare_prefix(const char* entry, const char* key, size_t key_len) { + int cmp = strncmp(entry, key, key_len); + if (cmp != 0) + return cmp; + unsigned char next = (unsigned char)entry[key_len]; + if (next == '\0') + return -1; /* entry == key sorts before key + '/' */ + return (int)next - (int)'/'; +} + +bool str_sorted_array_has_child_prefix(const StrSortedArray* array, const char* key) { + if (!array || !key || array->count == 0 || key[0] == '\0') + return false; + size_t key_len = strlen(key); + size_t lo = 0; + size_t hi = array->count; + while (lo < hi) { + size_t mid = lo + (hi - lo) / 2; + if (str_sorted_array_compare_prefix(array->items[mid], key, key_len) < 0) + lo = mid + 1; + else + hi = mid; + } + if (lo >= array->count) + return false; + const char* entry = array->items[lo]; + return strncmp(entry, key, key_len) == 0 && entry[key_len] == '/'; +} + +bool path_index_build(PathIndex* index, const char* const* entries, size_t count) { + if (!index) + return false; + index->exact.slots = NULL; + index->exact.capacity = 0; + index->exact.size = 0; + index->sorted.items = NULL; + index->sorted.count = 0; + if (!str_hash_set_init(&index->exact, count)) + return false; + if (!str_sorted_array_build(&index->sorted, entries, count)) { + str_hash_set_free(&index->exact); + return false; + } + for (size_t i = 0; i < count; i++) { + if (!str_hash_set_insert_ref(&index->exact, entries[i])) { + path_index_free(index); + return false; + } + } + return true; +} + +void path_index_free(PathIndex* index) { + if (!index) + return; + str_hash_set_free(&index->exact); + str_sorted_array_free(&index->sorted); +} + +bool path_index_contains(const PathIndex* index, const char* path) { + return index && str_hash_set_lookup(&index->exact, path); +} + +bool path_index_contains_n(const PathIndex* index, const char* path, size_t len) { + return index && str_hash_set_lookup_n(&index->exact, path, len); +} + +bool path_index_has_descendant(const PathIndex* index, const char* path) { + return index && str_sorted_array_has_child_prefix(&index->sorted, path); } char* output_escape(const char* string, bool eight_bit_output) { @@ -344,38 +430,23 @@ bool format_human_bytes(unsigned long long bytes, char* buffer, size_t buffer_si return written >= 0 && (size_t)written < buffer_size; } -/* Build the keep-set index: every manifest entry is inserted as an exact entry - and every ancestor directory prefix of it as a non-entry node. A lookup of - `rel` therefore succeeds iff `rel` is a kept file, a kept directory, or an - ancestor directory of kept content (the old is_dir_in_manifest predicate); - the entry flag distinguishes an exact kept file from a mere prefix. */ -static bool build_keep_index(ArrayList* manifest, StrHashSet* index) { - if (!str_hash_set_init(index, manifest && manifest->size > 0 ? (size_t)manifest->size : 1)) - return false; - if (!manifest) - return true; - for (int i = 0; i < manifest->size; i++) { - const char* entry = (const char*)manifest->items[i]; - if (!str_hash_set_insert_ref(index, entry, true)) - goto fail; - for (const char* slash = entry; (slash = strchr(slash, '/')) != NULL; slash++) { - if (!str_hash_set_insert_copy_n(index, entry, (size_t)(slash - entry), false)) - goto fail; - } - } - return true; -fail: - str_hash_set_free(index); - return false; +/* Build the keep-set index from the exact manifest entries only. A lookup of + `rel` succeeds iff `rel` is a kept entry, a kept directory, or an ancestor + directory of kept content (the old is_dir_in_manifest predicate); the sorted + view answers "is an ancestor of kept content" without materializing any + per-component prefix copy, so the index is O(manifest size) memory. */ +static bool build_keep_index(const ArrayList* manifest, PathIndex* index) { + if (!manifest || manifest->size <= 0) + return path_index_build(index, NULL, 0); + return path_index_build(index, (const char* const*)manifest->items, (size_t)manifest->size); } -static bool keep_is_dir(const StrHashSet* index, const char* rel_path) { - return str_hash_set_lookup(index, rel_path, NULL); +static bool keep_is_dir(const PathIndex* index, const char* rel_path) { + return path_index_contains(index, rel_path) || path_index_has_descendant(index, rel_path); } -static bool keep_is_file(const StrHashSet* index, const char* rel_path) { - bool is_entry = false; - return str_hash_set_lookup(index, rel_path, &is_entry) && is_entry; +static bool keep_is_file(const PathIndex* index, const char* rel_path) { + return path_index_contains(index, rel_path); } /* True when child_rel is, or lies below, a protected entry. A prefix "a" @@ -405,7 +476,7 @@ bool path_under_skip_prefix(const char* child_rel, bool at_root, const DeleteSki prefixes) mark the enclosing directory as surviving, exactly as they would make a real rmdir fail with ENOTEMPTY. Stops early once *count reaches the cap (sets *exceeds). Returns false on a traversal error. */ -static bool count_extras_fd(int dirfd, const char* rel_path, const StrHashSet* keep, size_t cap, +static bool count_extras_fd(int dirfd, const char* rel_path, const PathIndex* keep, size_t cap, size_t* count, bool* exceeds, const DeleteSkipEntry* skips, int skip_count, bool* survives) { /* openat(dirfd, ".") opens an independent file description: a dup() would @@ -495,7 +566,7 @@ static bool count_extras_fd(int dirfd, const char* rel_path, const StrHashSet* k return operation_ok; } -static bool delete_extras_fd(int dirfd, const char* rel_path, const StrHashSet* keep, +static bool delete_extras_fd(int dirfd, const char* rel_path, const PathIndex* keep, size_t max_delete, size_t* deleted_count, const DeleteSkipEntry* skips, int skip_count) { /* Independent file description (see count_extras_fd). */ @@ -595,7 +666,7 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, const StrHashSet* return operation_ok; } -DeleteWalkResult delete_extras_limited(const char* dest_root, ArrayList* manifest, +DeleteWalkResult delete_extras_limited(const char* dest_root, const ArrayList* manifest, size_t max_delete, const DeleteSkipEntry* skips, int skip_count, size_t* deleted_out) { if (deleted_out) @@ -604,7 +675,7 @@ DeleteWalkResult delete_extras_limited(const char* dest_root, ArrayList* manifes return DELETE_WALK_ERROR; /* Index the keep-set once so both passes answer membership in O(path length) instead of scanning every manifest entry for every destination entry. */ - StrHashSet keep; + PathIndex keep; if (!build_keep_index(manifest, &keep)) return DELETE_WALK_ERROR; int rootfd; @@ -619,7 +690,7 @@ DeleteWalkResult delete_extras_limited(const char* dest_root, ArrayList* manifes rootfd = open(dest_root, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); } if (rootfd < 0) { - str_hash_set_free(&keep); + path_index_free(&keep); return DELETE_WALK_ERROR; } if (max_delete != SIZE_MAX) { @@ -632,12 +703,12 @@ DeleteWalkResult delete_extras_limited(const char* dest_root, ArrayList* manifes skip_count, &survives); if (!counted_ok) { close(rootfd); - str_hash_set_free(&keep); + path_index_free(&keep); return DELETE_WALK_ERROR; } if (exceeds) { close(rootfd); - str_hash_set_free(&keep); + path_index_free(&keep); return DELETE_WALK_LIMIT_EXCEEDED; } } @@ -645,13 +716,13 @@ DeleteWalkResult delete_extras_limited(const char* dest_root, ArrayList* manifes bool ok = delete_extras_fd(rootfd, "", &keep, max_delete, &deleted_count, skips, skip_count); if (close(rootfd) != 0) ok = false; - str_hash_set_free(&keep); + path_index_free(&keep); if (deleted_out) *deleted_out = deleted_count; return ok ? DELETE_WALK_OK : DELETE_WALK_ERROR; } -bool delete_extras(const char* dest_root, ArrayList* manifest) { +bool delete_extras(const char* dest_root, const ArrayList* manifest) { return delete_extras_limited(dest_root, manifest, SIZE_MAX, NULL, 0, NULL) == DELETE_WALK_OK; } diff --git a/src/shared/utils.h b/src/shared/utils.h index f27c0c0..e97c635 100644 --- a/src/shared/utils.h +++ b/src/shared/utils.h @@ -7,17 +7,15 @@ #include /* Small open-addressing string hash set used to turn quadratic membership - * scans into O(path length) lookups (the --delete keep-set and the + * scans into O(path length) exact-match lookups (the --delete keep-set and the * --files-from allow-set). Keys are hashed with xxHash64 (seed 0); collisions * are resolved by linear probing over a power-of-two table that grows at 75% - * load. Slots may either borrow a caller-owned key (insert_ref) or own an - * internal copy (insert_copy_n); owned copies are released by - * str_hash_set_free. The set is not thread-safe for mutation, but a fully - * built set supports concurrent read-only lookups. */ + * load. Keys are always borrowed from the caller and must outlive the set; the + * set never copies or owns keys, so indexing M entries costs O(M) memory. The + * set is not thread-safe for mutation, but a fully built set supports + * concurrent read-only lookups. */ typedef struct { const char* key; /* NULL marks an empty slot */ - bool owned; /* key is an internal copy that free() must release */ - bool is_entry; /* key was inserted as an exact entry, not just a prefix */ } StrHashSetSlot; typedef struct { @@ -30,16 +28,54 @@ typedef struct { * allocation failure. */ bool str_hash_set_init(StrHashSet* set, size_t hint); void str_hash_set_free(StrHashSet* set); -/* Insert a borrowed key (must outlive the set). A duplicate only upgrades - * is_entry. Returns false on allocation failure. */ -bool str_hash_set_insert_ref(StrHashSet* set, const char* key, bool is_entry); -/* Insert a copy of the first `len` bytes of `key` (which need not be - * NUL-terminated). Returns false on allocation failure. */ -bool str_hash_set_insert_copy_n(StrHashSet* set, const char* key, size_t len, bool is_entry); -/* Look up a NUL-terminated key / a key of `len` bytes. On a hit, optionally - * reports whether the stored key was inserted as an exact entry. */ -bool str_hash_set_lookup(const StrHashSet* set, const char* key, bool* is_entry); -bool str_hash_set_lookup_n(const StrHashSet* set, const char* key, size_t len, bool* is_entry); +/* Insert a borrowed key (must outlive the set). A duplicate is ignored. + * Returns false on allocation failure. */ +bool str_hash_set_insert_ref(StrHashSet* set, const char* key); +/* Look up a NUL-terminated key / a key of `len` bytes. */ +bool str_hash_set_lookup(const StrHashSet* set, const char* key); +bool str_hash_set_lookup_n(const StrHashSet* set, const char* key, size_t len); + +/* Sorted, non-owning view of NUL-terminated strings. Built from borrowed + * pointers (qsort), so indexing M entries costs O(M) memory and O(M log M) + * time; exact membership and ancestor-prefix existence are binary searches + * that never materialize a prefix copy. */ +typedef struct { + const char** items; /* sorted with strcmp; borrowed, never freed */ + size_t count; +} StrSortedArray; + +/* Build `array` over the borrowed `items`. Only the pointer array is copied, + * never the strings. Returns false on allocation failure. */ +bool str_sorted_array_build(StrSortedArray* array, const char* const* items, size_t count); +void str_sorted_array_free(StrSortedArray* array); +/* True when some item equals `key`. */ +bool str_sorted_array_contains(const StrSortedArray* array, const char* key); +/* True when some item starts with `key` followed by '/' (i.e. `key` is a proper + * ancestor directory of an item). Allocates nothing. */ +bool str_sorted_array_has_child_prefix(const StrSortedArray* array, const char* key); + +/* Read-only membership index over exact relative paths. `exact` answers + * O(path length) equality; `sorted` answers whether any indexed path lies + * strictly below a query directory. Both borrow their keys from the caller and + * no ancestor prefix is stored as a separate string, so an index over M entries + * is O(M) memory regardless of path depth. Not thread-safe to build, but safe + * for concurrent read-only queries once built. */ +typedef struct { + StrHashSet exact; + StrSortedArray sorted; +} PathIndex; + +/* Build an index borrowing `entries` (which must outlive the index). Returns + * false on allocation failure, freeing any partial state. */ +bool path_index_build(PathIndex* index, const char* const* entries, size_t count); +void path_index_free(PathIndex* index); +/* True when `path` is an indexed entry. */ +bool path_index_contains(const PathIndex* index, const char* path); +/* Length-bounded form of path_index_contains (`path` need not be terminated). */ +bool path_index_contains_n(const PathIndex* index, const char* path, size_t len); +/* True when some indexed entry lies strictly below `path` (starts with + * `path` + '/'). */ +bool path_index_has_descendant(const PathIndex* index, const char* path); char* str_dup(const char* string); char* output_escape(const char* string, bool eight_bit_output); @@ -83,10 +119,10 @@ bool path_under_skip_prefix(const char* child_rel, bool at_root, const DeleteSki and the delete pass are two separate walks, so a concurrent change between them (another process adding/removing entries) can make the second pass delete a different set than the first one counted. */ -DeleteWalkResult delete_extras_limited(const char* dest_root, ArrayList* manifest, +DeleteWalkResult delete_extras_limited(const char* dest_root, const ArrayList* manifest, size_t max_delete, const DeleteSkipEntry* skips, int skip_count, size_t* deleted_out); -bool delete_extras(const char* dest_root, ArrayList* manifest); +bool delete_extras(const char* dest_root, const ArrayList* manifest); bool utils_set_authorized_root(int fd, const char* canonical_path); /* The fd-only compatibility form is fail-closed for path-based operations; * callers should use utils_set_authorized_root with the canonical identity. */ diff --git a/tests/test_file_list.c b/tests/test_file_list.c index bf31db5..80a0ca8 100644 --- a/tests/test_file_list.c +++ b/tests/test_file_list.c @@ -118,6 +118,79 @@ static void test_membership_matches_reference() { EXPECT_TRUE(file_list_affects(NULL, NULL)); } +/* Explicit ancestor/descendant coverage: a query that is a proper ancestor of + a listed entry is affected, and a query below a listed entry is affected, + while a component-boundary neighbor is not. */ +static void test_ancestor_and_descendant_queries() { + const char* path = "test_file_list_ancestor.txt"; + char err[160]; + write_list(path, "top/mid/leaf.txt\nsingle.txt\n"); + FileListSet* set = file_list_load(path, false, err, sizeof(err)); + EXPECT_NOT_NULL(set); + + /* q is an ancestor of a listed entry. */ + EXPECT_TRUE(file_list_affects(set, "top")); + EXPECT_TRUE(file_list_affects(set, "top/mid")); + EXPECT_FALSE(file_list_affects(set, "top/other")); /* neither direction */ + EXPECT_FALSE(file_list_affects(set, "to")); /* component boundary */ + + /* A listed entry is an ancestor of q. */ + EXPECT_TRUE(file_list_affects(set, "single.txt")); + EXPECT_TRUE(file_list_affects(set, "single.txt/deeper")); + EXPECT_FALSE(file_list_affects(set, "single.txtx")); /* boundary */ + + check_queries(set, + (const char*[]){"top", "top/mid", "top/mid/leaf.txt", "top/other", "single.txt", + "single.txt/deeper", "single.txtx", "to"}, + 8); + file_list_destroy(set); + remove(path); +} + +/* Regression for the remote OOM: an adversarial --files-from entry made of a + very deep chain of repeated components must be indexed with memory + proportional to the entry count. The old implementation stored one copied + ancestor prefix per component (O(L^2) bytes for a single entry); the sorted + index stores the exact entries only. */ +static void test_deep_paths_are_bounded() { + const char* path = "test_file_list_deep.txt"; + enum { COMPONENTS = 20000 }; + size_t entry_len = (size_t)COMPONENTS * 2; /* "a/" per component */ + char* entry = malloc(entry_len + 1); + EXPECT_NOT_NULL(entry); + for (size_t i = 0; i < entry_len; i += 2) { + entry[i] = 'a'; + entry[i + 1] = '/'; + } + entry[entry_len - 1] = 'z'; /* .../a/z: a deep leaf name */ + entry[entry_len] = '\0'; + + FILE* fp = fopen(path, "wb"); + EXPECT_NOT_NULL(fp); + EXPECT_EQ_INT((int)fwrite(entry, 1, entry_len, fp), (int)entry_len); + EXPECT_EQ_INT(fputc('\n', fp), '\n'); + fclose(fp); + + char err[160]; + FileListSet* set = file_list_load(path, false, err, sizeof(err)); + EXPECT_NOT_NULL(set); + EXPECT_EQ_INT(set->count, 1); + /* One exact entry stored, not one node per path component. */ + EXPECT_EQ_INT((int)set->index.sorted.count, 1); + EXPECT_EQ_INT((int)set->index.exact.size, 1); + EXPECT_TRUE(file_list_affects(set, entry)); /* exact */ + EXPECT_TRUE(file_list_affects(set, "a")); /* ancestor of the entry */ + EXPECT_TRUE(file_list_affects(set, "a/a")); /* deeper ancestor */ + EXPECT_FALSE(file_list_affects(set, "b")); /* unrelated */ + EXPECT_FALSE(file_list_affects(set, "aa")); /* component boundary */ + + file_list_destroy(set); + remove(path); + free(entry); +} + void test_file_list() { test_membership_matches_reference(); + test_ancestor_and_descendant_queries(); + test_deep_paths_are_bounded(); } diff --git a/tests/test_shared_utils.c b/tests/test_shared_utils.c index 6663695..c89c006 100644 --- a/tests/test_shared_utils.c +++ b/tests/test_shared_utils.c @@ -456,7 +456,72 @@ static void test_fd_peer_ip() { EXPECT_EQ_STR(peer_string, ""); } +/* The keep/files-from indexes must store exactly the input entries (one node + each), never a copied ancestor prefix per component. This builds a PathIndex + over paths thousands of components deep and checks the structural bound plus + the exact / descendant query semantics. */ +static void test_path_index_bounded() { + enum { COUNT = 8, COMPONENTS = 5000 }; + size_t entry_len = (size_t)COMPONENTS * 2 + 2; /* trailing "xN" */ + char* storage = malloc((size_t)COUNT * (entry_len + 1)); + EXPECT_NOT_NULL(storage); + const char** entries = calloc(COUNT, sizeof(char*)); + EXPECT_NOT_NULL(entries); + for (int i = 0; i < COUNT; i++) { + char* entry = storage + (size_t)i * (entry_len + 1); + size_t pos = 0; + for (int c = 0; c < COMPONENTS; c++) { + entry[pos++] = 'a'; + entry[pos++] = '/'; + } + entry[pos++] = 'x'; + entry[pos++] = (char)('0' + i); + entry[pos] = '\0'; + entries[i] = entry; + } + + PathIndex index; + EXPECT_TRUE(path_index_build(&index, entries, COUNT)); + EXPECT_EQ_INT((int)index.sorted.count, COUNT); + EXPECT_EQ_INT((int)index.exact.size, COUNT); + EXPECT_TRUE(path_index_contains(&index, entries[0])); + EXPECT_FALSE(path_index_contains(&index, "a")); + EXPECT_TRUE(path_index_has_descendant(&index, "a")); + EXPECT_TRUE(path_index_has_descendant(&index, "a/a")); + EXPECT_FALSE(path_index_has_descendant(&index, "aa")); + path_index_free(&index); + + free((void*)entries); + free(storage); +} + +static void test_path_index_semantics() { + const char* entries[] = {"a/b/c.txt", "a/b/d.txt", "x.txt", "deep/deeper/deepest"}; + PathIndex index; + EXPECT_TRUE(path_index_build(&index, entries, 4)); + EXPECT_TRUE(path_index_contains(&index, "a/b/c.txt")); + EXPECT_FALSE(path_index_contains(&index, "a/b")); + EXPECT_TRUE(path_index_contains_n(&index, "a/b/c.txt/ignored", 9)); + EXPECT_FALSE(path_index_contains_n(&index, "a/b/c.txt/ignored", 10)); + EXPECT_TRUE(path_index_has_descendant(&index, "a")); + EXPECT_TRUE(path_index_has_descendant(&index, "a/b")); + EXPECT_FALSE(path_index_has_descendant(&index, "a/b/c.txt")); + EXPECT_FALSE(path_index_has_descendant(&index, "ab")); + EXPECT_FALSE(path_index_has_descendant(&index, "")); + path_index_free(&index); + + /* A zero-entry index answers no queries. */ + PathIndex empty; + EXPECT_TRUE(path_index_build(&empty, NULL, 0)); + EXPECT_EQ_INT((int)empty.sorted.count, 0); + EXPECT_FALSE(path_index_contains(&empty, "a")); + EXPECT_FALSE(path_index_has_descendant(&empty, "a")); + path_index_free(&empty); +} + void test_shared_utils() { + test_path_index_bounded(); + test_path_index_semantics(); test_walker_removes_extras_keeps_manifest_and_protected(); test_walker_keeps_nested_manifest_dirs(); test_walker_max_delete_exceeded_deletes_nothing();