Re: [PATCH 5/6] test-hashmap: simplify alloc_test_entry

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 14, 2018, 19:01 UTC
Message-ID: <xmqq3723o0pf.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180214180819.GE9919@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> This function takes two ptr/len pairs, which implies that
> they can be arbitrary buffers. But internally, it assumes
> that each "ptr" is NUL-terminated at "len" (because we
> memcpy an extra byte to pick up the NUL terminator).

Makes quite a lot of sense. In addition, get_value() assumes that the key cannot have NUL in it, so the use of counted string in alloc_test_entry() serves no purpose other than to confuse the readers ;-)

> Note that we can get rid of the "l1" and "l2" variables from
> cmd_main() as a further cleanup, since they are now mostly

Made me wonder if this "we can" is "we could if we wanted to in the future although we do not go that far in this patch", but it turns out that this is also done, and I think it is a good clean-up.