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

Re: [PATCH v2 00/19] hashmap bug/safety/ease-of-use fixes

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Sep 26, 2019, 08:39 UTC
Message-ID
<nycvar.QRO.7.76.6.1909261032520.15067@tvgsbejvaqbjf.bet>
In-Reply-To
<20190924010324.22619-1-e@80x24.org>
Hi Eric,
On Tue, 24 Sep 2019, Eric Wong wrote:
Show 11 quoted lines
> Patches 1-11 are largely unchanged from the original series with the
> exception of 2, which is new and posted at:
>
> 	https://public-inbox.org/git/20190908074953.kux7zz4y7iolqko4@whir/
>
> 12-17 take further steps to get us away from hashmap_entry being
> the first element, but they're also a bit ugly because __typeof__
> isn't portable
>
> 18-19 finally brings me to the APIs I want to expose without
> relying on __typeof :)

I won't have time to review this patch series, but I wanted to throw out the idea of storing the offset in `struct hashmap`, i.e. the offset of the `struct hashmap_entry` in the actual entry struct?

Example:
	struct erics_entry {
		const char *i_want_this_to_be_the_first_field;
		struct hashmap_entry e;
		[...]
	};
	[...]
	struct erics_entry *dummy = NULL;
	size_t offset = ((char *)dummy->e) - ((char *)dummy);
	[... store that offset in the hashmap and subtract it from the
	stored pointers before returning an entry...]

IOW rather than assuming that it is the first field, we could allow an offset other than 0. I'm just not sure how to implement this elegantly, but it should be much easier to do this portably than the `typeof()` approach.

Ciao, Dscho

Show 131 quoted lines
>
> Apologies for the delays, been busy with other stuff...
>
> Previous discussion starts at:
>
> 	https://public-inbox.org/git/20190826024332.3403-1-e@80x24.org/
>
> The following changes since commit 745f6812895b31c02b29bdfe4ae8e5498f776c26:
>
>   First batch after Git 2.23 (2019-08-22 12:41:04 -0700)
>
> are available in the Git repository at:
>
>   https://80x24.org/git-svn.git hashmap-wip-v2
>
> for you to fetch changes up to 212a596edd99169b284912b7b70b6280e2fb90e6:
>
>   hashmap: remove type arg from hashmap_{get,put,remove}_entry (2019-09-24 00:42:22 +0000)
>
> ----------------------------------------------------------------
> Eric Wong (19):
>       diff: use hashmap_entry_init on moved_entry.ent
>       coccicheck: detect hashmap_entry.hash assignment
>       packfile: use hashmap_entry in delta_base_cache_entry
>       hashmap_entry_init takes "struct hashmap_entry *"
>       hashmap_get_next takes "const struct hashmap_entry *"
>       hashmap_add takes "struct hashmap_entry *"
>       hashmap_get takes "const struct hashmap_entry *"
>       hashmap_remove takes "const struct hashmap_entry *"
>       hashmap_put takes "struct hashmap_entry *"
>       introduce container_of macro
>       hashmap_get_next returns "struct hashmap_entry *"
>       hashmap: use *_entry APIs to wrap container_of
>       hashmap_get{,_from_hash} return "struct hashmap_entry *"
>       hashmap_cmp_fn takes hashmap_entry params
>       hashmap: use *_entry APIs for iteration
>       hashmap: hashmap_{put,remove} return hashmap_entry *
>       hashmap: introduce hashmap_free_entries
>       OFFSETOF_VAR macro to simplify hashmap iterators
>       hashmap: remove type arg from hashmap_{get,put,remove}_entry
>
>  attr.c                              |  22 ++---
>  blame.c                             |  25 +++---
>  builtin/describe.c                  |  21 +++--
>  builtin/difftool.c                  |  56 ++++++------
>  builtin/fast-export.c               |  15 ++--
>  builtin/fetch.c                     |  30 ++++---
>  config.c                            |  24 +++---
>  contrib/coccinelle/hashmap.cocci    |  16 ++++
>  diff.c                              |  31 ++++---
>  diffcore-rename.c                   |  15 ++--
>  git-compat-util.h                   |  33 ++++++++
>  hashmap.c                           |  58 ++++++++-----
>  hashmap.h                           | 164 +++++++++++++++++++++++++++++-------
>  merge-recursive.c                   |  87 ++++++++++---------
>  name-hash.c                         |  57 +++++++------
>  oidmap.c                            |  20 +++--
>  oidmap.h                            |   6 +-
>  packfile.c                          |  22 +++--
>  patch-ids.c                         |  18 ++--
>  range-diff.c                        |  10 +--
>  ref-filter.c                        |  31 ++++---
>  refs.c                              |  23 +++--
>  remote.c                            |  21 +++--
>  revision.c                          |  28 +++---
>  sequencer.c                         |  44 ++++++----
>  sub-process.c                       |  20 +++--
>  sub-process.h                       |   4 +-
>  submodule-config.c                  |  52 +++++++-----
>  t/helper/test-hashmap.c             |  49 ++++++-----
>  t/helper/test-lazy-init-name-hash.c |  12 +--
>  30 files changed, 647 insertions(+), 367 deletions(-)
>  create mode 100644 contrib/coccinelle/hashmap.cocci
>
> Eric Wong (19):
>   diff: use hashmap_entry_init on moved_entry.ent
>   coccicheck: detect hashmap_entry.hash assignment
>   packfile: use hashmap_entry in delta_base_cache_entry
>   hashmap_entry_init takes "struct hashmap_entry *"
>   hashmap_get_next takes "const struct hashmap_entry *"
>   hashmap_add takes "struct hashmap_entry *"
>   hashmap_get takes "const struct hashmap_entry *"
>   hashmap_remove takes "const struct hashmap_entry *"
>   hashmap_put takes "struct hashmap_entry *"
>   introduce container_of macro
>   hashmap_get_next returns "struct hashmap_entry *"
>   hashmap: use *_entry APIs to wrap container_of
>   hashmap_get{,_from_hash} return "struct hashmap_entry *"
>   hashmap_cmp_fn takes hashmap_entry params
>   hashmap: use *_entry APIs for iteration
>   hashmap: hashmap_{put,remove} return hashmap_entry *
>   hashmap: introduce hashmap_free_entries
>   OFFSETOF_VAR macro to simplify hashmap iterators
>   hashmap: remove type arg from hashmap_{get,put,remove}_entry
>
>  attr.c                              |  22 ++--
>  blame.c                             |  25 +++--
>  builtin/describe.c                  |  21 ++--
>  builtin/difftool.c                  |  56 ++++++----
>  builtin/fast-export.c               |  15 ++-
>  builtin/fetch.c                     |  30 ++---
>  config.c                            |  24 ++--
>  contrib/coccinelle/hashmap.cocci    |  16 +++
>  diff.c                              |  31 +++---
>  diffcore-rename.c                   |  15 ++-
>  git-compat-util.h                   |  33 ++++++
>  hashmap.c                           |  58 ++++++----
>  hashmap.h                           | 164 +++++++++++++++++++++++-----
>  merge-recursive.c                   |  87 ++++++++-------
>  name-hash.c                         |  57 +++++-----
>  oidmap.c                            |  20 ++--
>  oidmap.h                            |   6 +-
>  packfile.c                          |  22 ++--
>  patch-ids.c                         |  18 +--
>  range-diff.c                        |  10 +-
>  ref-filter.c                        |  31 ++++--
>  refs.c                              |  23 +++-
>  remote.c                            |  21 ++--
>  revision.c                          |  28 +++--
>  sequencer.c                         |  44 +++++---
>  sub-process.c                       |  20 ++--
>  sub-process.h                       |   4 +-
>  submodule-config.c                  |  52 +++++----
>  t/helper/test-hashmap.c             |  49 +++++----
>  t/helper/test-lazy-init-name-hash.c |  12 +-
>  30 files changed, 647 insertions(+), 367 deletions(-)
>  create mode 100644 contrib/coccinelle/hashmap.cocci
>
>
> base-commit: 745f6812895b31c02b29bdfe4ae8e5498f776c26
>
Previous: Derrick StoleeNext: Eric Wong
Message 68 of 71 in “hashmap: bugfixes, safety fixes, and WIP improvements”
  1. 00/11 hashmap: bugfixes, safety fixes, and WIP improvementsEric Wong, Aug 26, 2019
  2. 01/11 diff: use hashmap_entry_init on moved_entry.entEric Wong, Aug 26, 2019
  3. Derrick StoleeAug 27, 2019
  4. 02/11 packfile: use hashmap_entry in delta_base_cache_entryEric Wong, Aug 26, 2019
  5. 03/11 hashmap_entry_init takes "struct hashmap_entry *"Eric Wong, Aug 26, 2019
  6. Derrick StoleeAug 27, 2019
  7. Johannes SchindelinAug 28, 2019
  8. Eric WongAug 30, 2019
  9. Johannes SchindelinSep 2, 2019
  10. 04/11 hashmap_entry: detect improper initializationEric Wong, Aug 26, 2019
  11. Johannes SchindelinAug 27, 2019
  12. Eric WongAug 27, 2019
  13. Junio C HamanoAug 27, 2019
  14. Johannes SchindelinAug 28, 2019
  15. Phillip WoodAug 28, 2019
  16. Eric WongAug 30, 2019
  17. 04/11 coccicheck: detect hashmap_entry.hash assignmentEric Wong, Sep 8, 2019
  18. Junio C HamanoSep 9, 2019
  19. 05/11 hashmap_get_next takes "const struct hashmap_entry *"Eric Wong, Aug 26, 2019
  20. 06/11 hashmap_add takes "struct hashmap_entry *"Eric Wong, Aug 26, 2019
  21. 07/11 hashmap_get takes "const struct hashmap_entry *"Eric Wong, Aug 26, 2019
  22. 08/11 hashmap_remove takes "const struct hashmap_entry *"Eric Wong, Aug 26, 2019
  23. 09/11 hashmap_put takes "struct hashmap_entry *"Eric Wong, Aug 26, 2019
  24. 10/11 introduce container_of macroEric Wong, Aug 26, 2019
  25. Derrick StoleeAug 27, 2019
  26. Phillip WoodAug 28, 2019
  27. Eric WongAug 30, 2019
  28. 11/11 hashmap_get_next returns "struct hashmap_entry *"Eric Wong, Aug 26, 2019
  29. Derrick StoleeAug 27, 2019
  30. Eric WongAug 30, 2019
  31. 00/19 hashmap bug/safety/ease-of-use fixesEric Wong, Sep 24, 2019
  32. 01/19 diff: use hashmap_entry_init on moved_entry.entEric Wong, Sep 24, 2019
  33. 02/19 coccicheck: detect hashmap_entry.hash assignmentEric Wong, Sep 24, 2019
  34. Derrick StoleeSep 25, 2019
  35. 03/19 packfile: use hashmap_entry in delta_base_cache_entryEric Wong, Sep 24, 2019
  36. 04/19 hashmap_entry_init takes "struct hashmap_entry *"Eric Wong, Sep 24, 2019
  37. Derrick StoleeSep 25, 2019
  38. 05/19 hashmap_get_next takes "const struct hashmap_entry *"Eric Wong, Sep 24, 2019
  39. 06/19 hashmap_add takes "struct hashmap_entry *"Eric Wong, Sep 24, 2019
  40. 07/19 hashmap_get takes "const struct hashmap_entry *"Eric Wong, Sep 24, 2019
  41. Derrick StoleeSep 25, 2019
  42. Eric WongSep 30, 2019
  43. 08/19 hashmap_remove takes "const struct hashmap_entry *"Eric Wong, Sep 24, 2019
  44. Derrick StoleeSep 25, 2019
  45. 09/19 hashmap_put takes "struct hashmap_entry *"Eric Wong, Sep 24, 2019
  46. 10/19 introduce container_of macroEric Wong, Sep 24, 2019
  47. Derrick StoleeSep 25, 2019
  48. Eric WongSep 30, 2019
  49. 11/19 hashmap_get_next returns "struct hashmap_entry *"Eric Wong, Sep 24, 2019
  50. Derrick StoleeSep 25, 2019
  51. 12/19 hashmap: use *_entry APIs to wrap container_ofEric Wong, Sep 24, 2019
  52. Derrick StoleeSep 25, 2019
  53. 13/19 hashmap_get{,_from_hash} return "struct hashmap_entry *"Eric Wong, Sep 24, 2019
  54. 14/19 hashmap_cmp_fn takes hashmap_entry paramsEric Wong, Sep 24, 2019
  55. 15/19 hashmap: use *_entry APIs for iterationEric Wong, Sep 24, 2019
  56. 16/19 hashmap: hashmap_{put,remove} return hashmap_entry *Eric Wong, Sep 24, 2019
  57. 17/19 hashmap: introduce hashmap_free_entriesEric Wong, Sep 24, 2019
  58. 18/19 OFFSETOF_VAR macro to simplify hashmap iteratorsEric Wong, Sep 24, 2019
  59. Junio C HamanoSep 30, 2019
  60. Junio C HamanoOct 4, 2019
  61. Eric WongOct 4, 2019
  62. Junio C HamanoOct 4, 2019
  63. Eric WongOct 4, 2019
  64. Junio C HamanoOct 6, 2019
  65. 19/19 hashmap: remove type arg from hashmap_{get,put,remove}_entryEric Wong, Sep 24, 2019
  66. Derrick StoleeSep 25, 2019
  67. Derrick StoleeSep 25, 2019
  68. Johannes SchindelinSep 26, 2019
  69. Eric WongSep 30, 2019
  70. Phillip WoodSep 26, 2019
  71. Junio C HamanoSep 29, 2019

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.