From: Jeff King <peff@peff.net>
Date: Wed, 05 Jul 2017 09:00:03 GMT
Subject: Re: [PATCHv3 2/3] patch-ids.c: use hashmap correctly
Message-ID: <20170705090003.ervwwantjliim3az@sigill.intra.peff.net>
In-Reply-To: <20170630191407.5381-3-sbeller@google.com>

On Fri, Jun 30, 2017 at 12:14:06PM -0700, Stefan Beller wrote:

> As eluded to in the previous patch, the code in patch-ids.c is using
> the hashmaps API wrong.
> 
> Luckily we do not have a bug, as all hashmap functionality that we use
> here (hashmap_get) passes through the keydata.  If hashmap_get_next were
> to be used, a bug would occur as that passes NULL for the key_data.
> 
> So instead use the hashmap API correctly and provide the caller required
> data in the compare function via the first argument that always gets
> passed and was setup via the hashmap_init function.

Reviewing this a bit late, but it looks good to me. And I think the
explanation above nicely covers what is going on (and why it isn't a
bug).

-Peff

PS I think you meant "alluded" in the first sentence, unless you really
   were trying to escape the previous patch. :)