From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Eric Wong <e@80x24.org>
Cc: Junio C Hamano <gitster@pobox.com>,
Derrick Stolee <stolee@gmail.com>,
Phillip Wood <phillip.wood@dunelm.org.uk>,
git@vger.kernel.org
Subject: Re: [PATCH v2 00/19] hashmap bug/safety/ease-of-use fixes
Date: Thu, 26 Sep 2019 10:39:43 +0200 (CEST) [thread overview]
Message-ID: <nycvar.QRO.7.76.6.1909261032520.15067@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <20190924010324.22619-1-e@80x24.org>
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@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
>
next prev parent reply other threads:[~2019-09-26 8:40 UTC|newest]
Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-26 2:43 [PATCH 00/11] hashmap: bugfixes, safety fixes, and WIP improvements Eric Wong
2019-08-26 2:43 ` [PATCH 01/11] diff: use hashmap_entry_init on moved_entry.ent Eric Wong
2019-08-27 13:31 ` Derrick Stolee
2019-08-26 2:43 ` [PATCH 02/11] packfile: use hashmap_entry in delta_base_cache_entry Eric Wong
2019-08-26 2:43 ` [PATCH 03/11] hashmap_entry_init takes "struct hashmap_entry *" Eric Wong
2019-08-27 13:35 ` Derrick Stolee
2019-08-28 15:01 ` Johannes Schindelin
2019-08-30 19:48 ` Eric Wong
2019-09-02 13:46 ` Johannes Schindelin
2019-08-26 2:43 ` [PATCH 04/11] hashmap_entry: detect improper initialization Eric Wong
2019-08-27 9:10 ` Johannes Schindelin
2019-08-27 9:49 ` Eric Wong
2019-08-27 22:16 ` Junio C Hamano
2019-08-28 15:04 ` Johannes Schindelin
2019-08-28 9:03 ` Phillip Wood
2019-08-30 19:52 ` Eric Wong
2019-09-08 7:49 ` [RFC 04/11] coccicheck: detect hashmap_entry.hash assignment Eric Wong
2019-09-09 18:15 ` Junio C Hamano
2019-08-26 2:43 ` [PATCH 05/11] hashmap_get_next takes "const struct hashmap_entry *" Eric Wong
2019-08-26 2:43 ` [PATCH 06/11] hashmap_add takes "struct " Eric Wong
2019-08-26 2:43 ` [PATCH 07/11] hashmap_get takes "const struct " Eric Wong
2019-08-26 2:43 ` [PATCH 08/11] hashmap_remove " Eric Wong
2019-08-26 2:43 ` [PATCH 09/11] hashmap_put takes "struct " Eric Wong
2019-08-26 2:43 ` [PATCH 10/11] introduce container_of macro Eric Wong
2019-08-27 14:49 ` Derrick Stolee
2019-08-28 9:11 ` Phillip Wood
2019-08-30 19:43 ` Eric Wong
2019-08-26 2:43 ` [PATCH 11/11] hashmap_get_next returns "struct hashmap_entry *" Eric Wong
2019-08-27 14:53 ` Derrick Stolee
2019-08-30 19:36 ` Eric Wong
2019-09-24 1:03 ` [PATCH v2 00/19] hashmap bug/safety/ease-of-use fixes Eric Wong
2019-09-24 1:03 ` [PATCH v2 01/19] diff: use hashmap_entry_init on moved_entry.ent Eric Wong
2019-09-24 1:03 ` [PATCH v2 02/19] coccicheck: detect hashmap_entry.hash assignment Eric Wong
2019-09-25 12:44 ` Derrick Stolee
2019-09-24 1:03 ` [PATCH v2 03/19] packfile: use hashmap_entry in delta_base_cache_entry Eric Wong
2019-09-24 1:03 ` [PATCH v2 04/19] hashmap_entry_init takes "struct hashmap_entry *" Eric Wong
2019-09-25 12:48 ` Derrick Stolee
2019-09-24 1:03 ` [PATCH v2 05/19] hashmap_get_next takes "const struct " Eric Wong
2019-09-24 1:03 ` [PATCH v2 06/19] hashmap_add takes "struct " Eric Wong
2019-09-24 1:03 ` [PATCH v2 07/19] hashmap_get takes "const struct " Eric Wong
2019-09-25 12:52 ` Derrick Stolee
2019-09-30 9:57 ` Eric Wong
2019-09-24 1:03 ` [PATCH v2 08/19] hashmap_remove " Eric Wong
2019-09-25 12:54 ` Derrick Stolee
2019-09-24 1:03 ` [PATCH v2 09/19] hashmap_put takes "struct " Eric Wong
2019-09-24 1:03 ` [PATCH v2 10/19] introduce container_of macro Eric Wong
2019-09-25 13:12 ` Derrick Stolee
2019-09-30 10:39 ` Eric Wong
2019-09-24 1:03 ` [PATCH v2 11/19] hashmap_get_next returns "struct hashmap_entry *" Eric Wong
2019-09-25 13:05 ` Derrick Stolee
2019-09-24 1:03 ` [PATCH v2 12/19] hashmap: use *_entry APIs to wrap container_of Eric Wong
2019-09-25 13:15 ` Derrick Stolee
2019-09-24 1:03 ` [PATCH v2 13/19] hashmap_get{,_from_hash} return "struct hashmap_entry *" Eric Wong
2019-09-24 1:03 ` [PATCH v2 14/19] hashmap_cmp_fn takes hashmap_entry params Eric Wong
2019-09-24 1:03 ` [PATCH v2 15/19] hashmap: use *_entry APIs for iteration Eric Wong
2019-09-24 1:03 ` [PATCH v2 16/19] hashmap: hashmap_{put,remove} return hashmap_entry * Eric Wong
2019-09-24 1:03 ` [PATCH v2 17/19] hashmap: introduce hashmap_free_entries Eric Wong
2019-09-24 1:03 ` [PATCH v2 18/19] OFFSETOF_VAR macro to simplify hashmap iterators Eric Wong
2019-09-30 16:58 ` Junio C Hamano
2019-10-04 1:18 ` Junio C Hamano
2019-10-04 2:51 ` Eric Wong
2019-10-04 3:29 ` Junio C Hamano
2019-10-04 17:26 ` Eric Wong
2019-10-06 0:04 ` Junio C Hamano
2019-09-24 1:03 ` [PATCH v2 19/19] hashmap: remove type arg from hashmap_{get,put,remove}_entry Eric Wong
2019-09-25 12:42 ` Derrick Stolee
2019-09-25 13:30 ` [PATCH v2 00/19] hashmap bug/safety/ease-of-use fixes Derrick Stolee
2019-09-26 8:39 ` Johannes Schindelin [this message]
2019-09-30 10:01 ` Eric Wong
2019-09-26 13:48 ` Phillip Wood
2019-09-29 9:22 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=nycvar.QRO.7.76.6.1909261032520.15067@tvgsbejvaqbjf.bet \
--to=johannes.schindelin@gmx.de \
--cc=e@80x24.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=phillip.wood@dunelm.org.uk \
--cc=stolee@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).