On Sun, Jun 23, 2024 at 12:50 AM Pali Rohár <pali@xxxxxxxxxx> wrote: > On Sunday 23 June 2024 00:43:17 Andy Shevchenko wrote: > > On Sat, Jun 22, 2024 at 6:43 PM Pali Rohár <pali@xxxxxxxxxx> wrote: > > > On Saturday 22 June 2024 16:20:15 Pali Rohár wrote: ... > > > Definition of the table can be simplified by defining a macro which > > > expand to those verbose parts which are being repeating, without need to > > > introduce something "new". E.g.: > > > > > > #define DELL_LIS3LV02D_DMI_ENTRY(product_name, i2c_addr) \ > > > { \ > > > .matches = { > > > DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), \ > > > DMI_MATCH(DMI_PRODUCT_NAME, product_name), \ > > > }, \ > > > .driver_data = (void *)(i2c_addr), \ > > > > I'm not against this as we have a lot of different examples similar to > > this (with maybe other types of ID tables). But what makes me worry is > > the use of (void *) here. Shouldn't it be (const void *) so we exclude > > the (potential) cases of dropping const qualifier? > > I do not know what is the best way here for casting short int to void*. > For me it looks strange if such casting is needed. Anyway I think that > in any case casting 16-bit short integer to const void* does not produce > different result as casting to plain (non-const) void*. It is not about > const qualifier but about integer-to-pointer cast, where is dropped > everything to that integer type. You missed the long-term issue with macros like this. If we ever go away from an integer to a real pointer, it will be easier to make such a mistake. using proper casting will prevent you from doing that. > > > > } > > > > > > static const struct dmi_system_id smo8800_lis3lv02d_devices[] = { > > > DELL_LIS3LV02D_DMI_ENTRY("Latitude E5250", 0x29), > > > DELL_LIS3LV02D_DMI_ENTRY("Latitude E5450", 0x29), > > > ... > > > { } > > > }; > > > > > > Any opinion about this? -- With Best Regards, Andy Shevchenko