Re: [PATCH] builtin/receive-pack: avoid generic function name hmac
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 5, 2020, 06:37 UTC
- Message-ID
- <xmqqsggec2g1.fsf@gitster.c.googlers.com>
- In-Reply-To
- <20200505054630.5821-1-carenas@gmail.com>
Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:
Show 15 quoted lines
> fabec2c5c3 (builtin/receive-pack: switch to use the_hash_algo, 2019-08-18) > renames hmac_sha1 to hmac, as it was updated to (optionally) use the hash > function used by git (which won't be sha1 in the future). > > hmac() is provided by NetBSD > 8 libc and conflicts as shown by : > > builtin/receive-pack.c:421:13: error: conflicting types for 'hmac' > static void hmac(unsigned char *out, > ^~~~ > In file included from ./git-compat-util.h:172:0, > from ./builtin.h:4, > from builtin/receive-pack.c:1: > /usr/include/stdlib.h:305:10: note: previous declaration of 'hmac' was here > ssize_t hmac(const char *, const void *, size_t, const void *, size_t, void *, > ^~~~
Including <stdlib.h> is allowed to contaminate the namespace this way?
At least https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/stdlib.h.html does not seem to list hmac() there.
Show 6 quoted lines
> While the conflict, posses the question of why are we even implementing our > own RFC 2104 complaint HMAC while alternatives are readily available, the > simplest solution is to make sure the name is not as generic so it would > conflict with an equivalent one from the system (or linked libraries); so > rename it again to hmac_hash to reflect it will use the git's defined hash > function.
I do not mind renaming ours, as it is harder to get the <stdlib.h> fixed on the NetBSD, but I would phrase s/equivalent/unrelated/ (or even "irrelevant"), as it is clearly not an "alternative" that is readily available outside NetBSD. Otherwise we would have found this a lot sooner (it used to be called hmac_sha1() and renamed to hmac() in August last year).
> --- > builtin/receive-pack.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-)
Missing sign-off. The patch text is trivially correct.
Show 22 quoted lines
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 2cc18bbffd..b1d939e7ed 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -418,7 +418,7 @@ static int copy_to_sideband(int in, int out, void *arg)
> return 0;
> }
>
> -static void hmac(unsigned char *out,
> +static void hmac_hash(unsigned char *out,
> const char *key_in, size_t key_len,
> const char *text, size_t text_len)
> {
> @@ -463,7 +463,7 @@ static char *prepare_push_cert_nonce(const char *path, timestamp_t stamp)
> unsigned char hash[GIT_MAX_RAWSZ];
>
> strbuf_addf(&buf, "%s:%"PRItime, path, stamp);
> - hmac(hash, buf.buf, buf.len, cert_nonce_seed, strlen(cert_nonce_seed));
> + hmac_hash(hash, buf.buf, buf.len, cert_nonce_seed, strlen(cert_nonce_seed));
> strbuf_release(&buf);
>
> /* RFC 2104 5. HMAC-SHA1-80 */