git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/5] sparse-index: refactor path_found()

From
Elijah Newren <newren@gmail.com>
Date
Jun 24, 2024, 22:13 UTC
Message-ID
<CABPp-BHnUpPgg_wP67q2eSB_j01urbEaPV2Dqk1L+gUfqbZtpA@mail.gmail.com>
In-Reply-To
<7c3b545ee5ea3a0e6686594afe582fa1a19929f6.1718899877.git.gitgitgadget@gmail.com>

On Thu, Jun 20, 2024 at 10:11 AM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 12 quoted lines
>
> From: Derrick Stolee <stolee@gmail.com>
>
> In advance of changing the behavior of path_found(), take all of the
> intermediate data values and group them into a single struct. This
> simplifies the method prototype as well as the initialization. Future
> changes can be made directly to the struct and method without changing
> the callers with this approach.
>
> Note that the clear_path_found_data() method is currently empty, as
> there is nothing to free. However, this will change in the future, so
> place the method and its callers for now.
I can't parse the second half of the final sentence.  What was meant here?
Show 121 quoted lines
> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
>  sparse-index.c | 45 +++++++++++++++++++++++++++++----------------
>  1 file changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/sparse-index.c b/sparse-index.c
> index e0457c87fff..de6e727f5c1 100644
> --- a/sparse-index.c
> +++ b/sparse-index.c
> @@ -439,8 +439,22 @@ void ensure_correct_sparsity(struct index_state *istate)
>                 ensure_full_index(istate);
>  }
>
> -static int path_found(const char *path, const char **dirname, size_t *dir_len,
> -                     int *dir_found)
> +struct path_found_data {
> +       const char *dirname;
> +       size_t dir_len;
> +       int dir_found;
> +};
> +
> +#define PATH_FOUND_DATA_INIT { \
> +       .dir_found = 1 \
> +}
> +
> +static void clear_path_found_data(struct path_found_data *data)
> +{
> +       return;
> +}
> +
> +static int path_found(const char *path, struct path_found_data *data)
>  {
>         struct stat st;
>         char *newdir;
> @@ -450,7 +464,7 @@ static int path_found(const char *path, const char **dirname, size_t *dir_len,
>          * If dirname corresponds to a directory that doesn't exist, and this
>          * path starts with dirname, then path can't exist.
>          */
> -       if (!*dir_found && !memcmp(path, *dirname, *dir_len))
> +       if (!data->dir_found && !memcmp(path, data->dirname, data->dir_len))
>                 return 0;
>
>         /*
> @@ -472,15 +486,16 @@ static int path_found(const char *path, const char **dirname, size_t *dir_len,
>          * If path starts with directory (which we already lstat'ed and found),
>          * then no need to lstat parent directory again.
>          */
> -       if (*dir_found && *dirname && memcmp(path, *dirname, *dir_len))
> +       if (data->dir_found && data->dirname &&
> +           memcmp(path, data->dirname, data->dir_len))
>                 return 0;
>
>         /* Free previous dirname, and cache path's dirname */
> -       *dirname = path;
> -       *dir_len = newdir - path + 1;
> +       data->dirname = path;
> +       data->dir_len = newdir - path + 1;
>
> -       tmp = xstrndup(path, *dir_len);
> -       *dir_found = !lstat(tmp, &st);
> +       tmp = xstrndup(path, data->dir_len);
> +       data->dir_found = !lstat(tmp, &st);
>         free(tmp);
>
>         return 0;
> @@ -488,9 +503,7 @@ static int path_found(const char *path, const char **dirname, size_t *dir_len,
>
>  static int clear_skip_worktree_from_present_files_sparse(struct index_state *istate)
>  {
> -       const char *last_dirname = NULL;
> -       size_t dir_len = 0;
> -       int dir_found = 1;
> +       struct path_found_data data = PATH_FOUND_DATA_INIT;
>
>         int path_count = 0;
>         int to_restart = 0;
> @@ -502,7 +515,7 @@ static int clear_skip_worktree_from_present_files_sparse(struct index_state *ist
>
>                 if (ce_skip_worktree(ce)) {
>                         path_count++;
> -                       if (path_found(ce->name, &last_dirname, &dir_len, &dir_found)) {
> +                       if (path_found(ce->name, &data)) {
>                                 if (S_ISSPARSEDIR(ce->ce_mode)) {
>                                         to_restart = 1;
>                                         break;
> @@ -516,14 +529,13 @@ static int clear_skip_worktree_from_present_files_sparse(struct index_state *ist
>                            "sparse_path_count", path_count);
>         trace2_region_leave("index", "clear_skip_worktree_from_present_files_sparse",
>                             istate->repo);
> +       clear_path_found_data(&data);
>         return to_restart;
>  }
>
>  static void clear_skip_worktree_from_present_files_full(struct index_state *istate)
>  {
> -       const char *last_dirname = NULL;
> -       size_t dir_len = 0;
> -       int dir_found = 1;
> +       struct path_found_data data = PATH_FOUND_DATA_INIT;
>
>         int path_count = 0;
>
> @@ -537,7 +549,7 @@ static void clear_skip_worktree_from_present_files_full(struct index_state *ista
>
>                 if (ce_skip_worktree(ce)) {
>                         path_count++;
> -                       if (path_found(ce->name, &last_dirname, &dir_len, &dir_found))
> +                       if (path_found(ce->name, &data))
>                                 ce->ce_flags &= ~CE_SKIP_WORKTREE;
>                 }
>         }
> @@ -546,6 +558,7 @@ static void clear_skip_worktree_from_present_files_full(struct index_state *ista
>                            "full_path_count", path_count);
>         trace2_region_leave("index", "clear_skip_worktree_from_present_files_full",
>                             istate->repo);
> +       clear_path_found_data(&data);
>  }
>
>  void clear_skip_worktree_from_present_files(struct index_state *istate)
> --
> gitgitgadget

It's surprising how much a simple, straightforward translation of the code can improve its readability; I should have done it this way all along.

Previous: Derrick Stolee via GitGitGadgetNext: Derrick Stolee
Message 6 of 44 in “sparse-index: improve clear_skip_worktree_from_present_files()”
  1. 0/5 sparse-index: improve clear_skip_worktree_from_present_files()Derrick Stolee via GitGitGadget, Jun 20, 2024
  2. 1/5 sparse-index: refactor skip worktree retry logicDerrick Stolee via GitGitGadget, Jun 20, 2024
  3. Elijah NewrenJun 24, 2024
  4. Derrick StoleeJun 26, 2024
  5. 2/5 sparse-index: refactor path_found()Derrick Stolee via GitGitGadget, Jun 20, 2024
  6. Elijah NewrenJun 24, 2024
  7. Derrick StoleeJun 26, 2024
  8. 3/5 sparse-index: use strbuf in path_found()Derrick Stolee via GitGitGadget, Jun 20, 2024
  9. Elijah NewrenJun 24, 2024
  10. 4/5 sparse-index: count lstat() callsDerrick Stolee via GitGitGadget, Jun 20, 2024
  11. Elijah NewrenJun 24, 2024
  12. 5/5 sparse-index: improve lstat caching of sparse pathsDerrick Stolee via GitGitGadget, Jun 20, 2024
  13. Elijah NewrenJun 24, 2024
  14. Junio C HamanoJun 25, 2024
  15. Derrick StoleeJun 26, 2024
  16. Elijah NewrenJun 28, 2024
  17. Junio C HamanoJun 20, 2024
  18. Derrick StoleeJun 20, 2024
  19. Junio C HamanoJun 20, 2024
  20. 0/5 sparse-index: improve clear_skip_worktree_from_present_files()Derrick Stolee via GitGitGadget, Jun 26, 2024
  21. 1/5 sparse-checkout: refactor skip worktree retry logicDerrick Stolee via GitGitGadget, Jun 26, 2024
  22. Junio C HamanoJun 27, 2024
  23. Elijah NewrenJun 28, 2024
  24. Derrick StoleeJun 28, 2024
  25. Junio C HamanoJun 28, 2024
  26. Elijah NewrenJun 28, 2024
  27. Derrick StoleeJun 28, 2024
  28. 2/5 sparse-index: refactor path_found()Derrick Stolee via GitGitGadget, Jun 26, 2024
  29. 3/5 sparse-index: use strbuf in path_found()Derrick Stolee via GitGitGadget, Jun 26, 2024
  30. 4/5 sparse-index: count lstat() callsDerrick Stolee via GitGitGadget, Jun 26, 2024
  31. 5/5 sparse-index: improve lstat caching of sparse pathsDerrick Stolee via GitGitGadget, Jun 26, 2024
  32. Junio C HamanoJun 27, 2024
  33. Derrick StoleeJun 28, 2024
  34. Junio C HamanoJun 27, 2024
  35. Elijah NewrenJun 28, 2024
  36. Derrick StoleeJun 28, 2024
  37. 0/5 sparse-index: improve clear_skip_worktree_from_present_files()Derrick Stolee via GitGitGadget, Jun 28, 2024
  38. 1/5 sparse-checkout: refactor skip worktree retry logicDerrick Stolee via GitGitGadget, Jun 28, 2024
  39. 2/5 sparse-index: refactor path_found()Derrick Stolee via GitGitGadget, Jun 28, 2024
  40. 3/5 sparse-index: use strbuf in path_found()Derrick Stolee via GitGitGadget, Jun 28, 2024
  41. 4/5 sparse-index: count lstat() callsDerrick Stolee via GitGitGadget, Jun 28, 2024
  42. 5/5 sparse-index: improve lstat caching of sparse pathsDerrick Stolee via GitGitGadget, Jun 28, 2024
  43. Elijah NewrenJun 28, 2024
  44. Junio C HamanoJun 28, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.