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

Re: [PATCH 12/16] mktree: use iterator struct to add tree entries to index

From
Patrick Steinhardt <ps@pks.im>
Date
Jun 12, 2024, 09:40 UTC
Message-ID
<ZmltI7HA7O4w2E-6@tanuki>
In-Reply-To
<94d6615d634c4f78c88d3e01abbb27f13f85828c.1718130288.git.gitgitgadget@gmail.com>
On Tue, Jun 11, 2024 at 06:24:44PM +0000, Victoria Dye via GitGitGadget wrote:
Show 37 quoted lines
> From: Victoria Dye <vdye@github.com>
> 
> Create 'struct tree_entry_iterator' to manage iteration through a 'struct
> tree_entry_array'. Using an iterator allows for conditional iteration; this
> functionality will be necessary in later commits when performing parallel
> iteration through multiple sets of tree entries.
> 
> Signed-off-by: Victoria Dye <vdye@github.com>
> ---
>  builtin/mktree.c | 40 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/builtin/mktree.c b/builtin/mktree.c
> index 12f68187221..bee359e9978 100644
> --- a/builtin/mktree.c
> +++ b/builtin/mktree.c
> @@ -137,6 +137,38 @@ static void sort_and_dedup_tree_entry_array(struct tree_entry_array *arr)
>  	QSORT_S(arr->entries, arr->nr, ent_compare, &ignore_mode);
>  }
>  
> +struct tree_entry_iterator {
> +	struct tree_entry *current;
> +
> +	/* private */
> +	struct {
> +		struct tree_entry_array *arr;
> +		size_t idx;
> +	} priv;
> +};
> +
> +static void init_tree_entry_iterator(struct tree_entry_iterator *iter,
> +				     struct tree_entry_array *arr)
> +{
> +	iter->priv.arr = arr;
> +	iter->priv.idx = 0;
> +	iter->current = 0 < arr->nr ? arr->entries[0] : NULL;
> +}
Nit: Same comment as before, I think these should rather be named
`tree_entry_iterator_init()` and `tree_entry_iterator_advance()`.
Show 13 quoted lines
> +/*
> + * Advance the tree entry iterator to the next entry in the array. If no entries
> + * remain, 'current' is set to NULL. Returns the previous 'current' value of the
> + * iterator.
> + */
> +static struct tree_entry *advance_tree_entry_iterator(struct tree_entry_iterator *iter)
> +{
> +	struct tree_entry *prev = iter->current;
> +	iter->current = (iter->priv.idx + 1) < iter->priv.arr->nr
> +			? iter->priv.arr->entries[++iter->priv.idx]
> +			: NULL;
> +	return prev;
> +}

I think it's somewhat confusing to have this return a different value than `current`. When I call `next()`, then I expect the iterator to return the next item. And after having called `next()`, I expect that the current value is the one that the previous call to `next()` has returned.

To avoid confusion, I'd propose to get rid of the `current` member altogether. It's not needed as we already save the current index and avoids the confusion.

Patrick
Previous: Victoria Dye via GitGitGadgetNext: Victoria Dye
Message 31 of 65 in “mktree: support more flexible usage”
  1. 00/16 mktree: support more flexible usageVictoria Dye via GitGitGadget, Jun 11, 2024
  2. 01/16 mktree: use OPT_BOOLVictoria Dye via GitGitGadget, Jun 11, 2024
  3. 02/16 mktree: rename treeent to tree_entryVictoria Dye via GitGitGadget, Jun 11, 2024
  4. Patrick SteinhardtJun 12, 2024
  5. 03/16 mktree: use non-static tree_entry arrayVictoria Dye via GitGitGadget, Jun 11, 2024
  6. Eric SunshineJun 11, 2024
  7. Patrick SteinhardtJun 12, 2024
  8. 04/16 update-index: generalize 'read_index_info'Victoria Dye via GitGitGadget, Jun 11, 2024
  9. Junio C HamanoJun 11, 2024
  10. 06/16 index-info.c: parse object type in provided in read_index_infoVictoria Dye via GitGitGadget, Jun 11, 2024
  11. Junio C HamanoJun 12, 2024
  12. 05/16 index-info.c: identify empty input lines in read_index_infoVictoria Dye via GitGitGadget, Jun 11, 2024
  13. Junio C HamanoJun 11, 2024
  14. Victoria DyeJun 18, 2024
  15. 07/16 mktree: use read_index_info to read stdin linesVictoria Dye via GitGitGadget, Jun 11, 2024
  16. Junio C HamanoJun 12, 2024
  17. Patrick SteinhardtJun 12, 2024
  18. Junio C HamanoJun 12, 2024
  19. 08/16 mktree: add a --literally optionVictoria Dye via GitGitGadget, Jun 11, 2024
  20. Junio C HamanoJun 12, 2024
  21. 09/16 mktree: validate paths more carefullyVictoria Dye via GitGitGadget, Jun 11, 2024
  22. Junio C HamanoJun 12, 2024
  23. Victoria DyeJun 12, 2024
  24. Junio C HamanoJun 12, 2024
  25. 10/16 mktree: overwrite duplicate entriesVictoria Dye via GitGitGadget, Jun 11, 2024
  26. Patrick SteinhardtJun 12, 2024
  27. Victoria DyeJun 12, 2024
  28. 11/16 mktree: create tree using an in-core indexVictoria Dye via GitGitGadget, Jun 11, 2024
  29. Patrick SteinhardtJun 12, 2024
  30. 12/16 mktree: use iterator struct to add tree entries to indexVictoria Dye via GitGitGadget, Jun 11, 2024
  31. Patrick SteinhardtJun 12, 2024
  32. Victoria DyeJun 13, 2024
  33. 13/16 mktree: add directory-file conflict hashmapVictoria Dye via GitGitGadget, Jun 11, 2024
  34. 14/16 mktree: optionally add to an existing treeVictoria Dye via GitGitGadget, Jun 11, 2024
  35. Patrick SteinhardtJun 12, 2024
  36. Junio C HamanoJun 12, 2024
  37. Victoria DyeJun 17, 2024
  38. 15/16 mktree: allow deeper paths in inputVictoria Dye via GitGitGadget, Jun 11, 2024
  39. 16/16 mktree: remove entries when mode is 0Victoria Dye via GitGitGadget, Jun 11, 2024
  40. 00/17 mktree: support more flexible usageVictoria Dye via GitGitGadget, Jun 19, 2024
  41. 01/17 mktree: use OPT_BOOLVictoria Dye via GitGitGadget, Jun 19, 2024
  42. 02/17 mktree: rename treeent to tree_entryVictoria Dye via GitGitGadget, Jun 19, 2024
  43. 03/17 mktree: use non-static tree_entry arrayVictoria Dye via GitGitGadget, Jun 19, 2024
  44. 04/17 update-index: generalize 'read_index_info'Victoria Dye via GitGitGadget, Jun 19, 2024
  45. 05/17 index-info.c: return unrecognized lines to callerVictoria Dye via GitGitGadget, Jun 19, 2024
  46. 06/17 index-info.c: parse object type in provided in read_index_infoVictoria Dye via GitGitGadget, Jun 19, 2024
  47. 08/17 mktree.c: do not fail on mismatched submodule typeVictoria Dye via GitGitGadget, Jun 19, 2024
  48. 07/17 mktree: use read_index_info to read stdin linesVictoria Dye via GitGitGadget, Jun 19, 2024
  49. Junio C HamanoJun 20, 2024
  50. 09/17 mktree: add a --literally optionVictoria Dye via GitGitGadget, Jun 19, 2024
  51. 10/17 mktree: validate paths more carefullyVictoria Dye via GitGitGadget, Jun 19, 2024
  52. 11/17 mktree: overwrite duplicate entriesVictoria Dye via GitGitGadget, Jun 19, 2024
  53. Junio C HamanoJun 20, 2024
  54. 12/17 mktree: create tree using an in-core indexVictoria Dye via GitGitGadget, Jun 19, 2024
  55. Junio C HamanoJun 20, 2024
  56. 13/17 mktree: use iterator struct to add tree entries to indexVictoria Dye via GitGitGadget, Jun 19, 2024
  57. Junio C HamanoJun 26, 2024
  58. 14/17 mktree: add directory-file conflict hashmapVictoria Dye via GitGitGadget, Jun 19, 2024
  59. 15/17 mktree: optionally add to an existing treeVictoria Dye via GitGitGadget, Jun 19, 2024
  60. Junio C HamanoJun 26, 2024
  61. 16/17 mktree: allow deeper paths in inputVictoria Dye via GitGitGadget, Jun 19, 2024
  62. Junio C HamanoJun 27, 2024
  63. 17/17 mktree: remove entries when mode is 0Victoria Dye via GitGitGadget, Jun 19, 2024
  64. Junio C HamanoJun 25, 2024
  65. Junio C HamanoJul 10, 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.