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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Eric,

On Tue, 24 Sep 2019, Eric Wong wrote:

> 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

>
> Apologies for the delays, been busy with other stuff...
>
> Previous discussion starts at:
>
> 	https://public-inbox.org/git/20190826024332.3403-1-e@xxxxxxxxx/
>
> 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
>




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux