From: Alejandro Colomar <[email protected]>
Hi!
On 2023-08-18 10:46, Andy Shevchenko wrote:
> On Fri, Aug 18, 2023 at 01:28:42PM +0800, Herbert Xu wrote:
>> On Thu, Aug 17, 2023 at 04:33:17PM +0200, Lucas Segarra Fernandez wrote:
>>>
>>> +static struct pm_status_row pm_event_rows[] = {
>>> + PM_INFO_REGSET_ENTRY32(event_log[0], EVENT0),
>>> + PM_INFO_REGSET_ENTRY32(event_log[1], EVENT1),
>>> + PM_INFO_REGSET_ENTRY32(event_log[2], EVENT2),
>>> + PM_INFO_REGSET_ENTRY32(event_log[3], EVENT3),
>>> + PM_INFO_REGSET_ENTRY32(event_log[4], EVENT4),
>>> + PM_INFO_REGSET_ENTRY32(event_log[5], EVENT5),
>>> + PM_INFO_REGSET_ENTRY32(event_log[6], EVENT6),
>>> + PM_INFO_REGSET_ENTRY32(event_log[7], EVENT7),
>>> +};
>>> +
>>> +static_assert(ARRAY_SIZE_OF_FIELD(struct icp_qat_fw_init_admin_pm_info, event_log) ==
>>> + ARRAY_SIZE(pm_event_rows));
How about the following?
static_assert(ARRAY_SIZE(pm_event_rows) ==
ARRAY_SIZE(memberof(struct icp_qat_fw_init_admin_pm_info, event_log)));
It would only need one macro addition, without significant churn. It's
even less typing. Below is a scissor patch with the addition of
memberof().
I tried building the kernel, and didn't see any warnings form the patch
below.
Cheers,
Alex
>>
>> Was all of that churn just for this one line?
>>
>> How about simply declaring a macro
>>
>> #define QAT_NUMBER_OF_PM_EVENTS 8
>>
>> and then use it for the two arrays:
>>
>> static struct pm_status_row pm_event_rows[QAT_NUMBER_OF_PM_EVENTS] = {
>>
>> __u32 event_log[QAT_NUMBER_OF_PM_EVENTS];
>>
>> What am I missing?
>
> Splitting ARRAY_SIZE() is very beneficial on its own.
> The static assert is slightly more robust for the big code then defining
> something that at some point can be missed or miscalculated. Yet we can
> survive with a macro if you thinks it's better.
>
-----8<------------
Many xxxof_{member,field}() macros make use of the same construction to
refer to a member of a struct without needing a variable of the
structure type.
memberof(T, m) simplifies all of those, avoids possible mistakes in
repetition, adds a meaningful name to the construction, and improves
readability by avoiding too many parentheses together.
It uses a compound literal, which should optimized out by the compiler.
It's a bit simpler to read than the dereference of a casted null
pointer, due to having less parentheses in the implementation.
Cc: Andy Shevchenko <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: Lucas Segarra Fernandez <[email protected]>
Cc: Giovanni Cabiddu <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/container_of.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 713890c867be..5e762025c780 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -5,7 +5,9 @@
#include <linux/build_bug.h>
#include <linux/stddef.h>
-#define typeof_member(T, m) typeof(((T*)0)->m)
+
+#define memberof(T, member) ((T){}.member)
+#define typeof_member(T, m) typeof(memberof(T, m))
/**
* container_of - cast a member of a structure out to the containing structure
--
2.40.1
Hi Andy,
On 2023-08-21 13:18, Andy Shevchenko wrote:
> On Sun, Aug 20, 2023 at 09:52:22PM +0200, Alejandro Colomar wrote:
>>
>> -#define typeof_member(T, m) typeof(((T*)0)->m)
>> +
>> +#define memberof(T, member) ((T){}.member)
>
> I'm not sure. This seems to me utilization of compound literal, while above
> uses direct struct member pointer calculations.
Both can be used in most cases. The only exception is offsetof(3), where
you need the pointer calculation. The good thing about the compound
literal is that it's farther away from causing UB, but if that's not a
concern --using sizeof() or typeof() will usually make things safe from
UB, as there's really no dereference, but just to be a little paranoic--,
I could change the definition of memberof() to use the pointer thing.
Should I send a v2 with the pointer thing?
[I'll take some time, as I need to restore my USB with keys, which just
died yesterday. I didn't sign this email due to that.]
Cheers,
Alex
>
>> +#define typeof_member(T, m) typeof(memberof(T, m))
>
--
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5