index: replace map macros with inline functions - #5351
Conversation
|
I've amended another commit that goes into the same direction, but fixes a real bug where we'd always try to resize index maps twice on case-insensitive systems by accident. |
| GIT_INLINE(int) index_map_set(git_index *idx, git_idxmap *map, git_index_entry *e) | ||
| { | ||
| if (!map) | ||
| map = idx->entries_map; |
There was a problem hiding this comment.
I found it a little odd that it took an idx and a map and did this switch. Maybe splitting this into two functions?
int index_map_set(git_idxmap *map, git_index_entry *e, bool ignore_case)
and
int index_entry_set(git_index *idx, git_index_entry *e) which calls into that? Names provisional, of course.
I have the same feelings about index_map_resize...
There was a problem hiding this comment.
Yeah, I know, I was pondering the same. I guess I'll just go with index_map_set, not even introducing a second index_entry_set. The additional boilerplate isn't even worth it to skip a single parameter, if you ask me. Will update in a few minutes
Traditionally, our maps were mostly implemented via macros that had weird call semantics. This shows in our index code, where we have macros that insert into an index map case-sensitively or insensitively, as they still return error codes via an error parameter. This is unwieldy and, most importantly, not necessary anymore, due to the introduction of our high-level map API and removal of macros. Replace them with inlined functions to make code easier to read.
Depending on whether the index map is case-sensitive or insensitive, we need to call either `git_idxmap_icase_resize` or `git_idxmap_resize`. There are multiple locations where we thus use the following pattern: if (index->ignore_case && git_idxmap_icase_resize(map, length) < 0) return -1; else if (git_idxmap_resize(map, length) < 0) return -1; The funny thing is: on case-insensitive systems, we will try to resize the map twice in case where `git_idxmap_icase_resize()` doesn't error. While this will still use the correct hashing function as both map types use the same, this bug will at least cause us to resize the map twice in a row. Fix the issue by introducing a new function `index_map_resize` that handles case-sensitivity, similar to how `index_map_set` and `index_map_delete`. Convert all call sites where we were previously resizing the map to use that new function.
a859fff to
7fc97eb
Compare
Traditionally, our maps were mostly implemented via macros that had
weird call semantics. This shows in our index code, where we have macros
that insert into an index map case-sensitively or insensitively, as they
still return error codes via an error parameter. This is unwieldy and,
most importantly, not necessary anymore, due to the introduction of our
high-level map API and removal of macros.
Replace them with inlined functions to make code easier to read.