Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: git/git
base: 847af43a3afb39394d5fe58192f94b993ca18f9f
Choose a base ref
...
head repository: git/git
compare: d51d8cc36831bdabbbcec8553a7e83d9f5a3be4d
Choose a head ref
  • 7 commits
  • 7 files changed
  • 1 contributor

Commits on Apr 3, 2024

  1. reftable/basics: fix return type of binsearch() to be size_t 

    The `binsearch()` function can be used to find the first element for
which a callback functions returns a truish value. But while the array
size is of type `size_t`, the function in fact returns an `int` that is
supposed to index into that array.

Fix the function signature to return a `size_t`. This conversion does
not change any semantics given that the function would only ever return
a value in the range `[0, sz]` anyway.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
    @pks-t @gitster
    pks-t authored and gitster committed Apr 3, 2024
    Configuration menu
    Copy the full SHA
    3e7b36d View commit details
    Browse the repository at this point in the history

  2. reftable/basics: improve binsearch() test 

    The `binsearch()` test is somewhat weird in that it doesn't explicitly
spell out its expectations. Instead it does so in a rather ad-hoc way
with some hard-to-understand computations.

Refactor the test to spell out the needle as well as expected index for
all testcases. This refactoring highlights that the `binsearch_func()`
is written somewhat weirdly to find the first integer smaller than the
needle, not smaller or equal to it. Adjust the function accordingly.

While at it, rename the callback function to better convey its meaning.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
    @pks-t @gitster
    pks-t authored and gitster committed Apr 3, 2024
    Configuration menu
    Copy the full SHA
    e8b8082 View commit details
    Browse the repository at this point in the history

  3. reftable/refname: refactor binary search over refnames 

    It is comparatively hard to understand how exactly the binary search
over refnames works given that the function and variable names are not
exactly easy to grasp. Rename them to make this more obvious. This
should not result in any change in behaviour.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
    @pks-t @gitster
    pks-t authored and gitster committed Apr 3, 2024
    Configuration menu
    Copy the full SHA
    2176792 View commit details
    Browse the repository at this point in the history

  4. reftable/block: refactor binary search over restart points 

    When seeking a record in our block reader we perform a binary search
over the block's restart points so that we don't have to do a linear
scan over the whole block. The logic to do so is quite intricate though,
which makes it hard to understand.

Improve documentation and rename some of the functions and variables so
that the code becomes easier to understand overall. This refactoring
should not result in any change in behaviour.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
    @pks-t @gitster
    pks-t authored and gitster committed Apr 3, 2024
    Configuration menu
    Copy the full SHA
    77307a6 View commit details
    Browse the repository at this point in the history

  5. reftable/block: fix error handling when searching restart points 

    When doing the binary search over restart points in a block we need to
decode the record keys. This decoding step can result in an error when
the block is corrupted, which we indicate to the caller of the binary
search by setting `args.error = 1`. But the only caller that exists
mishandles this because it in fact performs the error check before
calling `binsearch()`.

Fix this bug by checking for errors at the right point in time.
Furthermore, refactor `binsearch()` so that it aborts the search in case
the callback function returns a negative value so that we don't
needlessly continue to search the block.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
    @pks-t @gitster
    pks-t authored and gitster committed Apr 3, 2024
    Configuration menu
    Copy the full SHA
    f9e8854 View commit details
    Browse the repository at this point in the history

  6. reftable/record: extract function to decode key lengths 

    We're about to refactor the binary search over restart points so that it
does not need to fully decode the record keys anymore. To do so we will
need to decode the record key lengths, which is non-trivial logic.

Extract the logic to decode these lengths from `refatble_decode_key()`
so that we can reuse it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
    @pks-t @gitster
    pks-t authored and gitster committed Apr 3, 2024
    Configuration menu
    Copy the full SHA
    cd75790 View commit details
    Browse the repository at this point in the history

  7. reftable/block: avoid decoding keys when searching restart points 

    When searching over restart points in a block we decode the key of each
of the records, which results in a memory allocation. This is quite
pointless though given that records it restart points will never use
prefix compression and thus store their keys verbatim in the block.

Refactor the code so that we can avoid decoding the keys, which saves us
some allocations.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
    @pks-t @gitster
    pks-t authored and gitster committed Apr 3, 2024
    Configuration menu
    Copy the full SHA
    d51d8cc View commit details
    Browse the repository at this point in the history