Re: [PATCH 10/10] hashmap_for_each_entry(): work around MSVC's run-time check failure #3
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 25, 2020, 18:38 UTC
- Message-ID
- <xmqq8scxln10.fsf@gitster.c.googlers.com>
- In-Reply-To
- <dc46d39611df4ebd90d9308364d887e638c1bc30.1601044119.git.gitgitgadget@gmail.com>
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 5 quoted lines
> From: Johannes Schindelin <johannes.schindelin@gmx.de> > > When compiling Git in Visual C, we do not have the luxury of > support for `typeof()`, and therefore `OFFSETOF_VAR()` unfortunately > has to fall back to pointer arithmetic.
Sigh.
Short of changing the signature of hashmap_put_entry() and friends to also take the type of these variables, I do not see any kosher way to reimplement the users of OFFSETOF_VAR() to help compilers without typeof() offhand. As a one-time annotation, the unfortunate noise we see in this patch may be tolerable, but what may make this approach unsustainable is that average programmers would not know, without compiling with that particular compiler, if their new variable that points at a hash_entry needs to have an oterwise unnecessary initialization. Also the variables that are left uninitialized by this patch may later require such an initialization.
Of course, it does not help that this workaround relies on an undefined behaviour, as you pointed out.
> When compiling code using the `hashmap_for_each_entry()` macro in Debug > mode, this leads to the "run-time check failure #3" because the variable > passed as `var` are not initialized, yet we calculate the pointer > difference `&(var->member)-var`.
Whoa, wait. If it is just that macro, can we perhaps do something like the attached patch?
Show 9 quoted lines
> > This "run-time check failure" causes a scary dialog to pop up. > > Work around this by initializing the respective variables. > > Note: according to the C standard, performing pointer arithmetic > with `NULL` is not exactly well-defined, but it seems to work > here, and it is at least better than performing pointer arithmetic > with an uninitialized pointer.
hashmap.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hashmap.h b/hashmap.h index ef220de4c6..49cd8a8e92 100644 --- a/hashmap.h +++ b/hashmap.h @@ -449,7 +449,7 @@ static inline struct hashmap_entry *hashmap_iter_first(struct hashmap *map, * containing a @member which is a "struct hashmap_entry" */ #define hashmap_for_each_entry(map, iter, var, member) \ - for (var = hashmap_iter_first_entry_offset(map, iter, \ + for (var = NULL, var = hashmap_iter_first_entry_offset(map, iter, \ OFFSETOF_VAR(var, member)); \ var; \ var = hashmap_iter_next_entry_offset(iter, \