| Message ID | 20190924010324.22619-11-e@80x24.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,01/19] diff: use hashmap_entry_init on moved_entry.ent | expand |
On 9/23/2019 9:03 PM, Eric Wong wrote: > This macro is popular within the Linux kernel for supporting > intrusive data structures such as linked lists, red-black trees, > and chained hash tables while allowing the compiler to do > type checking. > > I intend to use this to remove the limitation of "hashmap_entry" > being location-dependent and to allow more compile-time type > checking. nit: I don't know why the first-person singular language caused me to stumble during this message. Perhaps the following rewrite would convey the same information: Later patches will use container_of() to remove the limitation of "hashmap_entry" being location-dependent. This will complete the transition to compile-time type checking for the hashmap API. > This macro already exists in our source as "list_entry" in > list.h and making "list_entry" an alias to "container_of" > as the Linux kernel has done is a possibility. If it is the same code, then I would prefer you do this conversion now so we can see that equivalence in the patch AND we know that existing code will test it. Thanks, -Stolee > > Signed-off-by: Eric Wong <e@80x24.org> > --- > git-compat-util.h | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/git-compat-util.h b/git-compat-util.h > index 83be89de0a..4cc2c8283a 100644 > --- a/git-compat-util.h > +++ b/git-compat-util.h > @@ -1312,4 +1312,14 @@ void unleak_memory(const void *ptr, size_t len); > */ > #include "banned.h" > > +/* > + * container_of - Get the address of an object containing a field. > + * > + * @ptr: pointer to the field. > + * @type: type of the object. > + * @member: name of the field within the object. > + */ > +#define container_of(ptr, type, member) \ > + ((type *) ((char *)(ptr) - offsetof(type, member))) > + > #endif >
Derrick Stolee <stolee@gmail.com> wrote: > On 9/23/2019 9:03 PM, Eric Wong wrote: > > This macro is popular within the Linux kernel for supporting > > intrusive data structures such as linked lists, red-black trees, > > and chained hash tables while allowing the compiler to do > > type checking. > > > > I intend to use this to remove the limitation of "hashmap_entry" > > being location-dependent and to allow more compile-time type > > checking. > > nit: I don't know why the first-person singular language caused > me to stumble during this message. Perhaps the following rewrite > would convey the same information: > > Later patches will use container_of() to remove the limitation > of "hashmap_entry" being location-dependent. This will complete > the transition to compile-time type checking for the hashmap API. Agreed. Thanks. > > This macro already exists in our source as "list_entry" in > > list.h and making "list_entry" an alias to "container_of" > > as the Linux kernel has done is a possibility. > > If it is the same code, then I would prefer you do this conversion > now so we can see that equivalence in the patch AND we know that > existing code will test it. One problem is I'm not sure if list.h should have an #include for git-compat-util.h, since list.h comes from an outside source....
diff --git a/git-compat-util.h b/git-compat-util.h index 83be89de0a..4cc2c8283a 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -1312,4 +1312,14 @@ void unleak_memory(const void *ptr, size_t len); */ #include "banned.h" +/* + * container_of - Get the address of an object containing a field. + * + * @ptr: pointer to the field. + * @type: type of the object. + * @member: name of the field within the object. + */ +#define container_of(ptr, type, member) \ + ((type *) ((char *)(ptr) - offsetof(type, member))) + #endif
This macro is popular within the Linux kernel for supporting intrusive data structures such as linked lists, red-black trees, and chained hash tables while allowing the compiler to do type checking. I intend to use this to remove the limitation of "hashmap_entry" being location-dependent and to allow more compile-time type checking. This macro already exists in our source as "list_entry" in list.h and making "list_entry" an alias to "container_of" as the Linux kernel has done is a possibility. Signed-off-by: Eric Wong <e@80x24.org> --- git-compat-util.h | 10 ++++++++++ 1 file changed, 10 insertions(+)