git/refs/ref-cache.c, branch v2.51.2

ref-cache: use 'size_t' instead of int for length

2025-07-28T21:16:36Z

The commit 090eb5336c (refs: selectively set prefix in the seek functions, 2025-07-15) modified the ref-cache iterator to support seeking to a specified marker without setting the prefix. The commit adds and uses an integer 'len' to capture the length of the seek marker to compare with the entries of a given directory. Since the type of the variable is 'int', this is met with a typecast of converting a `strlen` to 'int' so it can be assigned to the 'len' variable. This is whole operation is a bit wrong: 1. Since the 'len' variable is eventually used in a 'strncmp', it should have been of type 'size_t'. 2. This also truncates the value provided from 'strlen' to an int, which could cause a large refname to produce a negative number. Let's do the correct thing here and simply use 'size_t' for `len`. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano

ref-cache: set prefix_state when seeking

2025-07-24T22:31:09Z

In 090eb5336c (refs: selectively set prefix in the seek functions, 2025-07-15) we separated the seeking functionality of reference iterators from the functionality to set prefix to an iterator. This allows users of ref iterators to seek to a particular reference to provide pagination support. The files-backend, uses the ref-cache iterator to iterate over loose refs. The iterator tracks directories and entries already processed via a stack of levels. Each level corresponds to a directory under the files backend. New levels are added to the stack, and when all entries from a level is yielded, the corresponding level is popped from the stack. To accommodate seeking, we need to populate and traverse the levels to stop the requested seek marker at the appropriate level and its entry index. Each level also contains a 'prefix_state' which is used for prefix matching, this allows the iterator to skip levels/entries which don't match a prefix. The default value of 'prefix_state' is PREFIX_CONTAINS_DIR, which yields all entries within a level. When purely seeking without prefix matching, we want to yield all entries. The commit however, skips setting the value explicitly. This causes the MemorySanitizer to issue a 'use-of-uninitialized-value' error when running 't/t6302-for-each-ref-filter'. Set the value explicitly to avoid to fix the issue. Reported-by: Kyle Lippincott Helped-by: Kyle Lippincott Helped-by: Jeff King Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano

refs: selectively set prefix in the seek functions

2025-07-15T18:54:20Z

The ref iterator exposes a `ref_iterator_seek()` function. The name suggests that this would seek the iterator to a specific reference in some ways similar to how `fseek()` works for the filesystem. However, the function actually sets the prefix for refs iteration. So further iteration would only yield references which match the particular prefix. This is a bit confusing. Let's add a 'flags' field to the function, which when set with the 'REF_ITERATOR_SEEK_SET_PREFIX' flag, will set the prefix for the iteration in-line with the existing behavior. Otherwise, the reference backends will simply seek to the specified reference and clears any previously set prefix. This allows users to start iteration from a specific reference. In the packed and reftable backend, since references are available in a sorted list, the changes are simply setting the prefix if needed. The changes on the files-backend are a little more involved, since the files backend uses the 'ref-cache' mechanism. We move out the existing logic within `cache_ref_iterator_seek()` to `cache_ref_iterator_set_prefix()` which is called when the 'REF_ITERATOR_SEEK_SET_PREFIX' flag is set. We then parse the provided seek string and set the required levels and their indexes to ensure that seeking is possible. Helped-by: Patrick Steinhardt Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano

ref-cache: remove unused function 'find_ref_entry()'

2025-07-15T18:54:19Z

The 'find_ref_entry' function is no longer used, so remove it. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano

refs/iterator: implement seeking for ref-cache iterators

2025-03-12T18:31:20Z

Implement seeking of ref-cache iterators. This is done by splitting most of the logic to seek iterators out of `cache_ref_iterator_begin()` and putting it into `cache_ref_iterator_seek()` so that we can reuse the logic. Note that we cannot use the optimization anymore where we return an empty ref iterator when there aren't any references, as otherwise it wouldn't be possible to reseek the iterator to a different prefix that may exist. This shouldn't be much of a performance concern though as we now start to bail out early in case `advance()` sees that there are no more directories to be searched. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

refs/iterator: separate lifecycle from iteration

2025-03-12T18:31:18Z

The ref and reflog iterators have their lifecycle attached to iteration: once the iterator reaches its end, it is automatically released and the caller doesn't have to care about that anymore. When the iterator should be released before it has been exhausted, callers must explicitly abort the iterator via `ref_iterator_abort()`. This lifecycle is somewhat unusual in the Git codebase and creates two problems: - Callsites need to be very careful about when exactly they call `ref_iterator_abort()`, as calling the function is only valid when the iterator itself still is. This leads to somewhat awkward calling patterns in some situations. - It is impossible to reuse iterators and re-seek them to a different prefix. This feature isn't supported by any iterator implementation except for the reftable iterators anyway, but if it was implemented it would allow us to optimize cases where we need to search for specific references repeatedly by reusing internal state. Detangle the lifecycle from iteration so that we don't deallocate the iterator anymore once it is exhausted. Instead, callers are now expected to always call a newly introduce `ref_iterator_free()` function that deallocates the iterator and its internal state. Note that the `dir_iterator` is somewhat special because it does not implement the `ref_iterator` interface, but is only used to implement other iterators. Consequently, we have to provide `dir_iterator_free()` instead of `dir_iterator_release()` as the allocated structure itself is managed by the `dir_iterator` interfaces, as well, and not freed by `ref_iterator_free()` like in all the other cases. While at it, drop the return value of `ref_iterator_abort()`, which wasn't really required by any of the iterator implementations anyway. Furthermore, stop calling `base_ref_iterator_free()` in any of the backends, but instead call it in `ref_iterator_free()`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

ref-cache: fix invalid free operation in `free_ref_entry`

2024-11-26T19:34:37Z

In cfd971520e (refs: keep track of unresolved reference value in iterators, 2024-08-09), we added a new field "referent" into the "struct ref" structure. In order to free the "referent", we unconditionally freed the "referent" by simply adding a "free" statement. However, this is a bad usage. Because when ref entry is either directory or loose ref, we will always execute the following statement: free(entry->u.value.referent); This does not make sense. We should never access the "entry->u.value" field when "entry" is a directory. However, the change obviously doesn't break the tests. Let's analysis why. The anonymous union in the "ref_entry" has two members: one is "struct ref_value", another is "struct ref_dir". On a 64-bit machine, the size of "struct ref_dir" is 32 bytes, which is smaller than the 48-byte size of "struct ref_value". And the offset of "referent" field in "struct ref_value" is 40 bytes. So, whenever we create a new "ref_entry" for a directory, we will leave the offset from 40 bytes to 48 bytes untouched, which means the value for this memory is zero (NULL). It's OK to free a NULL pointer, but this is merely a coincidence of memory layout. To fix this issue, we now ensure that "free(entry->u.value.referent)" is only called when "entry->flag" indicates that it represents a loose reference and not a directory to avoid the invalid memory operation. Signed-off-by: shejialuo Signed-off-by: Junio C Hamano

refs: keep track of unresolved reference value in iterators

2024-08-09T15:47:33Z

Since ref iterators do not hold onto the direct value of a reference without resolving it, the only way to get ahold of a direct value of a symbolic ref is to make a separate call to refs_read_symbolic_ref. To make accessing the direct value of a symbolic ref more efficient, let's save the direct value of the ref in the iterators for both the files backend and the reftable backend. Signed-off-by: John Cai Signed-off-by: Junio C Hamano

refs/files: fix NULL pointer deref when releasing ref store

2024-06-06T16:04:32Z

The `free_ref_cache()` function is not `NULL` safe and will thus segfault when being passed such a pointer. This can easily happen when trying to release a partially initialized "files" ref store. Fix this. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

refs: pass repo when peeling objects

2024-05-17T17:33:39Z

Both `peel_object()` and `peel_iterated_oid()` implicitly rely on `the_repository` to look up objects. Despite the fact that we want to get rid of `the_repository`, it also leads to some restrictions in our ref iterators when trying to retrieve the peeled value for a repository other than `the_repository`. Refactor these functions such that both take a repository as argument and remove the now-unnecessary restrictions. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano