Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 12, 2013, 18:35 UTC
- Message-ID
- <xmqqd2odq45y.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <20130912182057.GB32069@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 18 quoted lines
> On Thu, Sep 12, 2013 at 08:37:08AM -0700, Junio C Hamano wrote: > >> > I wonder if GCC has changed it's behaviour to more closely match C99. >> > Clang as a compatibility article about this sort of issue: >> > >> > http://clang.llvm.org/compatibility.html#inline >> >> Interesting. The ways the page suggests as fixes are >> >> - change it to a "statis inline"; >> - remove "inline" from the definition; >> - provide an external (non-inline) def somewhere else; >> - compile with gnu899 dialect. > > Right, option 3 seems perfectly reasonable to me, as we must be prepared > to cope with a decision not to inline the function, and there has to be > _some_ linked implementation. But shouldn't libc be providing an > external, linkable strcasecmp in this case?
That is exactly my point when I said that the third one is nonsense for a definition in the standard header file.
I think we would want something like below.
-- >8 -- Subject: [PATCH] mailmap: work around implementations with pure inline strcasecmp
On some systems, string.h has _only_ inline definition of strcasecmp without supplying a non-inline implementation anywhere, which is, eh, "unusual"; we cannot take an address of such a function to store it in namemap.cmp. Work it around by introducing our own level of indirection.
Signed-off-by: Junio C Hamano <gitster@pobox.com> --- mailmap.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/mailmap.c b/mailmap.c index 44614fc..8863e23 100644 --- a/mailmap.c +++ b/mailmap.c @@ -52,6 +52,19 @@ static void free_mailmap_entry(void *p, const char *s) string_list_clear_func(&me->namemap, free_mailmap_info); } +/* + * On some systems, string.h has _only_ inline definition of strcasecmp + * without supplying a non-inline implementation anywhere, which is, eh, + * "unusual"; we cannot take an address of such a function to store it in + * namemap.cmp. This is here as a workaround---do not assign strcasecmp + * directly to namemap.cmp until we know no systems that matter have such + * an "unusual" string.h. + */ +static int namemap_cmp(const char *a, const char *b) +{ + return strcasecmp(a, b); +} + static void add_mapping(struct string_list *map, char *new_name, char *new_email, char *old_name, char *old_email) @@ -75,7 +88,7 @@ static void add_mapping(struct string_list *map, item = string_list_insert_at_index(map, index, old_email); me = xcalloc(1, sizeof(struct mailmap_entry)); me->namemap.strdup_strings = 1; - me->namemap.cmp = strcasecmp; + me->namemap.cmp = namemap_cmp; item->util = me; } @@ -241,7 +254,7 @@ int read_mailmap(struct string_list *map, char **repo_abbrev) int err = 0; map->strdup_strings = 1; - map->cmp = strcasecmp; + map->cmp = namemap_cmp; if (!git_mailmap_blob && is_bare_repository()) git_mailmap_blob = "HEAD:.mailmap";
-- 1.8.4-485-gec42fe2