Re: [PATCH 1/2] oidmap: make oidmap_free independent of struct layout

From: Jeff King <peff@peff.net>
Date: Apr 8, 2020, 21:54 UTC
Message-ID: <20200408215423.GA3468797@coredump.intra.peff.net>
In-Reply-To: <20200408040659.14511-1-abhishekkumar8222@gmail.com>

On Wed, Apr 08, 2020 at 09:36:58AM +0530, Abhishek Kumar wrote:

Show 6 quoted lines

> c8e424c introduced hashmap_free_entries, which can free any struct
> pointer, regardless of the hashmap_entry field offset.
> 
> oidmap does not make use of this flexibilty, hardcoding the offset to
> zero instead. Let's fix this by passing struct type and member to
> hashmap_free_entries.

I'm not sure how much this improves anything.

We're now telling the hashmap code how to get to our "struct oidmap_entry" pointer by using the correct offset there. But we know that offset will always be 0, because we put our internal hashmap entry at the start of oidmap_entry.

What you can't do is:

  struct foo {
    int first_member;
    struct oidmap_entry not_the_first_member;
  };

and work directly with "struct foo" pointers. You have to convert oidmap_entry pointers into foo pointers with container_of() or similar. And that is true both before and after your patch.

That said, I don't mind doing this as a cleanup; there's a subtle dependency on the location of internal_entry within object_entry, and this would move towards getting rid of it.

-Peff