Subject: Re: [PATCH 10/11] introduce container_of macro

From: Phillip Wood <[email-masked]>
Date: Wed, 28 Aug 2019 10:11:42 +0100

On 27/08/2019 15:49, Derrick Stolee wrote:
> On 8/25/2019 10:43 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.
>>
>> 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.
> [snip]
>> +/*
>> + * 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
> 
> I think it would be good to include at least one use of this
> macro in this patch. As it stands, I need to look at the next
> patch to make sense of what this is doing.
> 
> It took me a little while to parse what is happening here.
> 'ptr' is a pointer to the generic struct (in our case,
> 'struct hashmap_entry *'), while 'type' is the parent type,
> and 'member' is the name of the member in 'type' that is
> of type typeof(*ptr).

It took me a couple of minutes to figure it out as well. The rest of 
this patch series adds some very welcome type safety changes, at first 
sight this patch threatens to undermine that as there is no check (and 
no compiler independent way to check) that type == typeof(*ptr). It 
would also be helpful if the commit message could explain how this can 
be used to improve type safety.

Best Wishes

Phillip

> Perhaps this is easier to grok for others than it was for me.
> 
> -Stolee
>