aboutsummaryrefslogtreecommitdiff
path: root/reftable/stack.c
AgeCommit message (Collapse)Author
13 daysreftable/system: add abstraction to retrieve time in millisecondsPatrick Steinhardt
We directly call gettimeofday(3p), which may not be available on some platforms. Provide the infrastructure to let projects easily use their own implementations of this function. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
13 daysreftable/stack: provide fsync(3p) via system headerPatrick Steinhardt
Users of the reftable library are expected to provide their own function callback in cases they want to sync(3p) data to disk via the reftable write options. But if no such function was provided we end up calling fsync(3p) directly, which may not even be available on some systems. While dropping the explicit call to fsync(3p) would work, it would lead to an unsafe default behaviour where a project may have forgotten to set up the callback function, and that could lead to potential data loss. So this is not a great solution. Instead, drop the callback function and make it mandatory for the project to define fsync(3p). In the case of Git, we can then easily inject our custom implementation via the "reftable-system.h" header so that we continue to use `fsync_component()`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-10reftable/stack: add function to check if optimization is requiredKarthik Nayak
The reftable backend performs auto-compaction as part of its regular flow, which is required to keep the number of tables part of a stack at bay. This allows it to stay optimized. Compaction can also be triggered voluntarily by the user via the 'git pack-refs' or the 'git refs optimize' command. However, currently there is no way for the user to check if optimization is required without actually performing it. Extract out the heuristics logic from 'reftable_stack_auto_compact()' into an internal function 'update_segment_if_compaction_required()'. Then use this to add and expose `reftable_stack_compaction_required()` which will allow users to check if the reftable backend can be optimized. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-10reftable/stack: return stack segments directlyKarthik Nayak
The `stack_table_sizes_for_compaction()` function returns individual sizes of each reftable table. This function is only called by `reftable_stack_auto_compact()` to decide which tables need to be compacted, if any. Modify the function to directly return the segments, which avoids the extra step of receiving the sizes only to pass it to `suggest_compaction_segment()`. A future commit will also add functionality for checking whether auto-compaction is necessary without performing it. This change allows code re-usability in that context. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-10-13Merge branch 'kn/reftable-consistency-checks'Junio C Hamano
The reftable backend learned to sanity check its on-disk data more carefully. * kn/reftable-consistency-checks: refs/reftable: add fsck check for checking the table name reftable: add code to facilitate consistency checks fsck: order 'fsck_msg_type' alphabetically Documentation/fsck-msgids: remove duplicate msg id reftable: check for trailing newline in 'tables.list' refs: move consistency check msg to generic layer refs: remove unused headers
2025-10-07reftable: check for trailing newline in 'tables.list'Karthik Nayak
In the reftable format, the 'tables.list' file contains a newline separated list of tables. While we parse this file, we do not check or care about the last newline. Tighten the parser in `parse_names()` to return an appropriate error if the last newline is missing. This requires modification to `parse_names()` to now return the error while accepting the output as a third argument. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-12reftable: don't second-guess errors from flock interfacePatrick Steinhardt
The `flock` interface is implemented as part of "reftable/system.c" and thus needs to be implemented by the integrator between the reftable library and its parent code base. As such, we cannot rely on any specific implementation thereof. Regardless of that, users of the `flock` subsystem rely on `errno` being set to specific values. This is fragile and not documented anywhere and doesn't really make for a good interface. Refactor the code so that the implementations themselves are expected to return reftable-specific error codes. Our implementation of the `flock` subsystem already knows to do this for all error paths except one. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-12reftable/stack: handle outdated stacks when compactingPatrick Steinhardt
When we compact the reftable stack we first acquire the lock for the "tables.list" file and then reload the stack to check that it is still up-to-date. This is done by calling `stack_uptodate()`, which knows to return zero in case the stack is up-to-date, a positive value if it is not and a negative error code on unexpected conditions. We don't do proper error checking though, but instead we only check whether the returned error code is non-zero. If so, we simply bubble it up the calling stack, which means that callers may see an unexpected positive value. Fix this issue by translating to `REFTABLE_OUTDATED_ERROR` instead. Handle this situation in `reftable_addition_commit()`, where we perform a best-effort auto-compaction. All other callsites of `stack_uptodate()` know to handle a positive return value and thus don't need to be fixed. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-12reftable/stack: allow passing flags to `reftable_stack_add()`Patrick Steinhardt
The `reftable_stack_add()` function is a simple wrapper to lock the stack, add records to it via a callback and then commit the result. One problem with it though is that it doesn't accept any flags for creating the addition. This makes it impossible to automatically reload the stack in case it was modified before we managed to lock the stack. Add a `flags` field to plug this gap and pass it through accordingly. For now this new flag won't be used by us, but it will be used by libgit2. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-12reftable/stack: fix compiler warning due to missing bracesPatrick Steinhardt
While perfectly legal, older compiler toolchains complain when zero-initializing structs that contain nested structs with `{0}`: /home/libgit2/source/deps/reftable/stack.c:862:35: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces] struct reftable_addition empty = REFTABLE_ADDITION_INIT; ^~~~~~~~~~~~~~~~~~~~~~ /home/libgit2/source/deps/reftable/stack.c:707:33: note: expanded from macro 'REFTABLE_ADDITION_INIT' #define REFTABLE_ADDITION_INIT {0} ^ We had the discussion around whether or not we want to handle such bogus compiler errors in the past already [1]. Back then we basically decided that we do not care about such old-and-buggy compilers, so while we could fix the issue by using `{{0}}` instead this is not the preferred way to handle this in the Git codebase. We have an easier fix though: we can just drop the macro altogether and handle initialization of the struct in `reftable_stack_addition_init()`. Callers are expected to call this function already, so this change even simplifies the calling convention. [1]: https://lore.kernel.org/git/20220710081135.74964-1-sunshine@sunshineco.com/T/ Suggested-by: Carlo Arenas <carenas@gmail.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-12reftable/stack: reorder code to avoid forward declarationsPatrick Steinhardt
We have a couple of forward declarations in the stack-related code of the reftable library. These declarations aren't really required, but are simply caused by unfortunate ordering. Reorder the code and remove the forward declarations. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-04-07reftable/constants: make block types part of the public interfacePatrick Steinhardt
Now that reftable blocks can be read individually via the public interface it becomes necessary for callers to be able to distinguish the different types of blocks. Expose the relevant constants. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-04-07reftable/reader: rename data structure to "table"Patrick Steinhardt
The `struct reftable_reader` subsystem encapsulates a table that has been read from the disk. As such, the current name of that structure is somewhat hard to understand as it only talks about the fact that we read something from disk, without really giving an indicator _what_ that is. Furthermore, this naming schema doesn't really fit well into how the other structures are named: `reftable_merged_table`, `reftable_stack`, `reftable_block` and `reftable_record` are all named after what they encapsulate. Rename the subsystem to `reftable_table`, which directly gives a hint that the data structure is about handling the individual tables part of the stack. While this change results in a lot of churn, it prepares for us exposing the APIs to third-party callers now that the reftable library is a standalone library that can be linked against by other projects. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-04-07reftable: fix formatting of the license headerPatrick Steinhardt
The license headers used across the reftable library doesn't follow our typical coding style for multi-line comments. Fix it. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-18reftable/stack: stop using `sleep_millisec()`Patrick Steinhardt
Refactor our use of `sleep_millisec()` by open-coding it with poll(3p), which is the current implementation of this function. Ideally, we'd use a more direct way to sleep, but there is no equivalent to sleep(3p) that would accept milliseconds as input. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-18reftable/system: introduce `reftable_rand()`Patrick Steinhardt
Introduce a new system-level `reftable_rand()` function that generates a single unsigned integer for us. The implementation of this function is to be provided by the calling codebase, which allows us to more easily hook into pre-seeded random number generators. Adapt the two callsites where we generated random data. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-18reftable/stack: stop using `write_in_full()`Patrick Steinhardt
Similar to the preceding commit, drop our use of `write_in_full()` and implement a new wrapper `reftable_write_full()` that handles this logic for us. This is done to reduce our dependency on the Git library. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-18reftable/stack: stop using `read_in_full()`Patrick Steinhardt
There is a single callsite of `read_in_full()` in the reftable library. Open-code the function to reduce our dependency on the Git library. Note that we only partially port over the logic from `read_in_full()` and its underlying `xread()` helper. Most importantly, the latter also knows to handle `EWOULDBLOCK` via `handle_nonblock()`. This logic is irrelevant for us though because the reftable library never sets the `O_NONBLOCK` option in the first place. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-14Merge branch 'kn/reflog-migration-fix-followup'Junio C Hamano
Code clean-up. * kn/reflog-migration-fix-followup: reftable: prevent 'update_index' changes after adding records refs: use 'uint64_t' for 'ref_update.index' refs: mark `ref_transaction_update_reflog()` as static
2025-01-28Merge branch 'ps/reftable-sign-compare'Junio C Hamano
The reftable/ library code has been made -Wsign-compare clean. * ps/reftable-sign-compare: reftable: address trivial -Wsign-compare warnings reftable/blocksource: adjust `read_block()` to return `ssize_t` reftable/blocksource: adjust type of the block length reftable/block: adjust type of the restart length reftable/block: adapt header and footer size to return a `size_t` reftable/basics: adjust `hash_size()` to return `uint32_t` reftable/basics: adjust `common_prefix_size()` to return `size_t` reftable/record: handle overflows when decoding varints reftable/record: drop unused `print` function pointer meson: stop disabling -Wsign-compare
2025-01-22reftable: prevent 'update_index' changes after adding recordsKarthik Nayak
The function `reftable_writer_set_limits()` allows updating the 'min_update_index' and 'max_update_index' of a reftable writer. These values are written to both the writer's header and footer. Since the header is written during the first block write, any subsequent changes to the update index would create a mismatch between the header and footer values. The footer would contain the newer values while the header retained the original ones. To protect against this bug, prevent callers from updating these values after any record is written. To do this, modify the function to return an error whenever the limits are modified after any record adds. Check for record adds within `reftable_writer_set_limits()` by checking the `last_key` and `next` variable. The former is updated after each record added, but is reset at certain points. The latter is set after writing the first block. Modify all callers of the function to anticipate a return type and handle it accordingly. Add a unit test to also ensure the function returns the error as expected. Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-01-21reftable: address trivial -Wsign-compare warningsPatrick Steinhardt
Address the last couple of trivial -Wsign-compare warnings in the reftable library and remove the DISABLE_SIGN_COMPARE_WARNINGS macro that we have in "reftable/system.h". Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-01-07reftable/stack: accept insecure random bytesPatrick Steinhardt
The reftable library uses randomness in two call paths: - When reading a stack in case some of the referenced tables disappears. The randomness is used to delay the next read by a couple of milliseconds. - When writing a new table, where the randomness gets appended to the table name (e.g. "0x000000000001-0x000000000002-0b1d8ddf.ref"). In neither of these cases do we need strong randomness. Unfortunately though, we have observed test failures caused by the former case. In t0610 we have a test that spawns a 100 processes at once, all of which try to write a new table to the stack. And given that all of the processes will require randomness, it can happen that these processes make the entropy pool run dry, which will then cause us to die: + test_seq 100 + printf %s commit\trefs/heads/branch-%s\n 68d032e9edd3481ac96382786ececc37ec28709e 1 + printf %s commit\trefs/heads/branch-%s\n 68d032e9edd3481ac96382786ececc37ec28709e 2 ... + git update-ref refs/heads/branch-98 HEAD + git update-ref refs/heads/branch-97 HEAD + git update-ref refs/heads/branch-99 HEAD + git update-ref refs/heads/branch-100 HEAD fatal: unable to get random bytes fatal: unable to get random bytes fatal: unable to get random bytes fatal: unable to get random bytes fatal: unable to get random bytes fatal: unable to get random bytes fatal: unable to get random bytes The report was for NonStop, which uses OpenSSL as the backend for randomness. In the preceding commit we have adapted that backend to also return randomness in case the entropy pool is empty and the caller passes the `CSPRNG_BYTES_INSECURE` flag. Do so to fix the issue. Reported-by: Randall S. Becker <rsbecker@nexbridge.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-01-07wrapper: allow generating insecure random bytesPatrick Steinhardt
The `csprng_bytes()` function generates randomness and writes it into a caller-provided buffer. It abstracts over a couple of implementations, where the exact one that is used depends on the platform. These implementations have different guarantees: while some guarantee to never fail (arc4random(3)), others may fail. There are two significant failures to distinguish from one another: - Systemic failure, where e.g. opening "/dev/urandom" fails or when OpenSSL doesn't have a provider configured. - Entropy failure, where the entropy pool is exhausted, and thus the function cannot guarantee strong cryptographic randomness. While we cannot do anything about the former, the latter failure can be acceptable in some situations where we don't care whether or not the randomness can be predicted. Introduce a new `CSPRNG_BYTES_INSECURE` flag that allows callers to opt into weak cryptographic randomness. The exact behaviour of the flag depends on the underlying implementation: - `arc4random_buf()` never returns an error, so it doesn't change. - `getrandom()` pulls from "/dev/urandom" by default, which never blocks on modern systems even when the entropy pool is empty. - `getentropy()` seems to block when there is not enough randomness available, and there is no way of changing that behaviour. - `GtlGenRandom()` doesn't mention anything about its specific failure mode. - The fallback reads from "/dev/urandom", which also returns bytes in case the entropy pool is drained in modern Linux systems. That only leaves OpenSSL with `RAND_bytes()`, which returns an error in case the returned data wouldn't be cryptographically safe. This function is replaced with a call to `RAND_pseudo_bytes()`, which can indicate whether or not the returned data is cryptographically secure via its return value. If it is insecure, and if the `CSPRNG_BYTES_INSECURE` flag is set, then we ignore the insecurity and return the data regardless. It is somewhat questionable whether we really need the flag in the first place, or whether we wouldn't just ignore the potentially-insecure data. But the risk of doing that is that we might have or grow callsites that aren't aware of the potential insecureness of the data in places where it really matters. So using a flag to opt-in to that behaviour feels like the more secure choice. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-28reftable: avoid leaks on realloc errorRené Scharfe
When realloc(3) fails, it returns NULL and keeps the original allocation intact. REFTABLE_ALLOC_GROW overwrites both the original pointer and the allocation count variable in that case, simultaneously leaking the original allocation and misrepresenting the number of storable items. parse_names() and reftable_buf_add() avoid leaking by restoring the original pointer value on failure, but all other callers seem to be OK with losing the old allocation. Add a new variant of the macro, REFTABLE_ALLOC_GROW_OR_NULL, which plugs the leak and zeros the allocation counter. Use it for those callers. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-23Merge branch 'ps/reftable-alloc-failures-zalloc-fix'Junio C Hamano
Recent reftable updates mistook a NULL return from a request for 0-byte allocation as OOM and died unnecessarily, which has been corrected. * ps/reftable-alloc-failures-zalloc-fix: reftable/basics: return NULL on zero-sized allocations reftable/stack: fix zero-sized allocation when there are no readers reftable/merged: fix zero-sized allocation when there are no readers reftable/stack: don't perform auto-compaction with less than two tables
2024-12-22reftable/stack: fix zero-sized allocation when there are no readersPatrick Steinhardt
Similar as the preceding commit, we may try to do a zero-sized allocation when reloading a reftable stack that ain't got any tables. It is implementation-defined whether malloc(3p) returns a NULL pointer in that case or a zero-sized object. In case it does return a NULL pointer though it causes us to think we have run into an out-of-memory situation, and thus we return an error. Fix this by only allocating arrays when they have at least one entry. Reported-by: Randall S. Becker <rsbecker@nexbridge.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-22reftable/stack: don't perform auto-compaction with less than two tablesPatrick Steinhardt
In order to compact tables we need at least two tables. Bail out early from `reftable_stack_auto_compact()` in case we have less than two tables. In the original, `stack_table_sizes_for_compaction()` yields an array that has the same length as the number of tables. This array is then passed on to `suggest_compaction_segment()`, which returns an empty segment in case we have less than two tables. The segment is then passed to `segment_size()`, which will return `0` because both start and end of the segment are `0`. And because we only call `stack_compact_range()` in case we have a positive segment size we don't perform auto-compaction at all. Consequently, this change does not result in a user-visible change in behaviour when called with a single table. But when called with no tables this protects us against a potential out-of-memory error: `stack_table_sizes_for_compaction()` would try to allocate a zero-byte object when there aren't any tables, and that may lead to a `NULL` pointer on some platforms like NonStop which causes us to bail out with an out-of-memory error. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-26reftable/stack: add mechanism to notify callers on reloadPatrick Steinhardt
Reftable stacks are reloaded in two cases: - When calling `reftable_stack_reload()`, if the stat-cache tells us that the stack has been modified. - When committing a reftable addition. While callers can figure out the second case, they do not have a mechanism to figure out whether `reftable_stack_reload()` led to an actual reload of the on-disk data. All they can do is thus to assume that data is always being reloaded in that case. Improve the situation by introducing a new `on_reload()` callback to the reftable options. If provided, the function will be invoked every time the stack has indeed been reloaded. This allows callers to invalidate data that depends on the current stack data. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-26reftable/stack: add accessor for the hash IDPatrick Steinhardt
Add an accessor function that allows callers to access the hash ID of a reftable stack. This function will be used in a subsequent commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-19reftable/system: provide thin wrapper for lockfile subsystemPatrick Steinhardt
We use the lockfile subsystem to write lockfiles for "tables.list". As with the tempfile subsystem, the lockfile subsystem also hooks into our infrastructure to prune stale locks via atexit(3p) or signal handlers. Furthermore, the lockfile subsystem also handles locking timeouts, which do add quite a bit of logic. Having to reimplement that in the context of Git wouldn't make a whole lot of sense, and it is quite likely that downstream users of the reftable library may have a better idea for how exactly to implement timeouts. So again, provide a thin wrapper for the lockfile subsystem instead such that the compatibility shim is fully self-contained. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-19reftable/stack: drop only use of `get_locked_file_path()`Patrick Steinhardt
We've got a single callsite where we call `get_locked_file_path()`. As we're about to convert our usage of the lockfile subsystem to instead be used via a compatibility shim we'd have to implement more logic for this single callsite. While that would be okay if Git was the only supposed user of the reftable library, it's a bit more awkward when considering that we have to reimplement this functionality for every user of the library eventually. Refactor the code such that we don't call `get_locked_file_path()` anymore. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-19reftable/system: provide thin wrapper for tempfile subsystemPatrick Steinhardt
We use the tempfile subsystem to write temporary tables, but given that we're in the process of converting the reftable library to become standalone we cannot use this subsystem directly anymore. While we could in theory convert the code to use mkstemp(3p) instead, we'd lose access to our infrastructure that automatically prunes tempfiles via atexit(3p) or signal handlers. Provide a thin wrapper for the tempfile subsystem instead. Like this, the compatibility shim is fully self-contained in "reftable/system.c". Downstream users of the reftable library would have to implement their own tempfile shims by replacing "system.c" with a custom version. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-19reftable/stack: stop using `fsync_component()` directlyPatrick Steinhardt
We're executing `fsync_component()` directly in the reftable library so that we can fsync data to disk depending on "core.fsync". But as we're in the process of converting the reftable library to become standalone we cannot use that function in the library anymore. Refactor the code such that users of the library can inject a custom fsync function via the write options. This allows us to get rid of the dependency on "write-or-die.h". Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-19reftable/system: stop depending on "hash.h"Patrick Steinhardt
We include "hash.h" in "reftable/system.h" such that we can use hash format IDs as well as the raw size of SHA1 and SHA256. As we are in the process of converting the reftable library to become standalone we of course cannot rely on those constants anymore. Introduce a new `enum reftable_hash` to replace internal uses of the hash format IDs and new constants that replace internal uses of the hash size. Adapt the reftable backend to set up the correct hash function. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-17reftable: handle trivial `reftable_buf` errorsPatrick Steinhardt
Convert the reftable library such that we handle failures with the new `reftable_buf` interfaces. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-17reftable/stack: adapt `stack_filename()` to handle allocation failuresPatrick Steinhardt
The `stack_filename()` function cannot pass any errors to the caller as it has a `void` return type. Adapt it and its callers such that we can handle errors and start handling allocation failures. There are two interesting edge cases in `reftable_stack_destroy()` and `reftable_addition_close()`. Both of these are trying to tear down their respective structures, and while doing so they try to unlink some of the tables they have been keeping alive. Any earlier attempts to do that may fail on Windows because it keeps us from deleting such tables while they are still open, and thus we re-try on close. It's okay and even expected that this can fail when the tables are still open by another process, so we handle the allocation failures gracefully and just skip over any file whose name we couldn't figure out. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-17reftable/stack: adapt `format_name()` to handle allocation failuresPatrick Steinhardt
The `format_name()` function cannot pass any errors to the caller as it has a `void` return type. Adapt it and its callers such that we can handle errors and start handling allocation failures. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-17reftable: convert from `strbuf` to `reftable_buf`Patrick Steinhardt
Convert the reftable library to use the `reftable_buf` interface instead of the `strbuf` interface. This is mostly a mechanical change via sed(1) with some manual fixes where functions for `strbuf` and `reftable_buf` differ. The converted code does not yet handle allocation failures. This will be handled in subsequent commits. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-17reftable: stop using `strbuf_addf()`Patrick Steinhardt
We're about to introduce our own `reftable_buf` type to replace `strbuf`. One function we'll have to convert is `strbuf_addf()`, which is used in a handful of places. This function uses `snprintf()` internally, which makes porting it a bit more involved: - It is not available on all platforms. - Some platforms like Windows have broken implementations. So by using `snprintf()` we'd also push the burden on downstream users of the reftable library to make available a properly working version of it. Most callsites of `strbuf_addf()` are trivial to convert to not using it. We do end up using `snprintf()` in our unit tests, but that isn't much of a problem for downstream users of the reftable library. While at it, remove a useless call to `strbuf_reset()` in `t_reftable_stack_auto_compaction_with_locked_tables()`. We don't write to the buffer before this and initialize it with `STRBUF_INIT`, so there is no need to reset anything. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-10Merge branch 'ps/reftable-alloc-failures'Junio C Hamano
The reftable library is now prepared to expect that the memory allocation function given to it may fail to allocate and to deal with such an error. * ps/reftable-alloc-failures: (26 commits) reftable/basics: fix segfault when growing `names` array fails reftable/basics: ban standard allocator functions reftable: introduce `REFTABLE_FREE_AND_NULL()` reftable: fix calls to free(3P) reftable: handle trivial allocation failures reftable/tree: handle allocation failures reftable/pq: handle allocation failures when adding entries reftable/block: handle allocation failures reftable/blocksource: handle allocation failures reftable/iter: handle allocation failures when creating indexed table iter reftable/stack: handle allocation failures in auto compaction reftable/stack: handle allocation failures in `stack_compact_range()` reftable/stack: handle allocation failures in `reftable_new_stack()` reftable/stack: handle allocation failures on reload reftable/reader: handle allocation failures in `reader_init_iter()` reftable/reader: handle allocation failures for unindexed reader reftable/merged: handle allocation failures in `merged_table_init_iter()` reftable/writer: handle allocation failures in `reftable_new_writer()` reftable/writer: handle allocation failures in `writer_index_hash()` reftable/record: handle allocation failures when decoding records ...
2024-10-02reftable: introduce `REFTABLE_FREE_AND_NULL()`Patrick Steinhardt
We have several calls to `FREE_AND_NULL()` in the reftable library, which of course uses free(3P). As the reftable allocators are pluggable we should rather call the reftable specific function, which is `reftable_free()`. Introduce a new macro `REFTABLE_FREE_AND_NULL()` and adapt the callsites accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable: fix calls to free(3P)Patrick Steinhardt
There are a small set of calls to free(3P) in the reftable library. As the reftable allocators are pluggable we should rather call the reftable specific function, which is `reftable_free()`. Convert the code accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable: handle trivial allocation failuresPatrick Steinhardt
Handle trivial allocation failures in the reftable library and its unit tests. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/stack: handle allocation failures in auto compactionPatrick Steinhardt
Handle allocation failures in `reftable_stack_auto_compact()`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/stack: handle allocation failures in `stack_compact_range()`Patrick Steinhardt
Handle allocation failures in `stack_compact_range()`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/stack: handle allocation failures in `reftable_new_stack()`Patrick Steinhardt
Handle allocation failures in `reftable_new_stack()`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/stack: handle allocation failures on reloadPatrick Steinhardt
Handle allocation failures in `reftable_stack_reload_once()`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/merged: handle allocation failures in `merged_table_init_iter()`Patrick Steinhardt
Handle allocation failures in `merged_table_init_iter()`. While at it, merge `merged_iter_init()` into the function. It only has a single caller and merging them makes it easier to handle allocation failures consistently. This change also requires us to adapt `reftable_stack_init_*_iterator()` to bubble up the new error codes of `merged_table_iter_init()`. Adapt callsites accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/writer: handle allocation failures in `reftable_new_writer()`Patrick Steinhardt
Handle allocation failures in `reftable_new_writer()`. Adapt the function to return an error code to return such failures. While at it, rename it to match our code style as we have to touch up every callsite anyway. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>