drivers/iio/accel/adxl372.c | 3 +-- drivers/iio/accel/msa311.c | 4 +--- drivers/iio/adc/dln2-adc.c | 4 +--- drivers/iio/adc/mt6360-adc.c | 3 +-- drivers/iio/adc/rockchip_saradc.c | 4 +--- drivers/iio/adc/rtq6056.c | 4 +--- drivers/iio/adc/stm32-adc.c | 3 +-- drivers/iio/adc/ti-ads1015.c | 4 +--- drivers/iio/adc/ti-ads1119.c | 4 +--- drivers/iio/adc/ti-lmp92064.c | 4 +--- drivers/iio/adc/ti-tsc2046.c | 3 +-- drivers/iio/chemical/scd30_core.c | 3 +-- drivers/iio/chemical/scd4x.c | 3 +-- drivers/iio/chemical/sunrise_co2.c | 6 ++---- drivers/iio/dac/ad3552r.c | 3 +-- drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 5 ++--- drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c | 5 ++--- drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 4 +--- drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 6 ++---- drivers/iio/light/bh1745.c | 4 +--- drivers/iio/light/ltr501.c | 4 +--- drivers/iio/light/opt4060.c | 4 +--- drivers/iio/light/veml6030.c | 4 +--- drivers/iio/magnetometer/af8133j.c | 4 +--- drivers/iio/pressure/bmp280-core.c | 5 +---- drivers/iio/pressure/mpl3115.c | 3 +-- drivers/iio/pressure/mprls0025pa_i2c.c | 5 +---- drivers/iio/pressure/zpa2326.c | 4 +--- drivers/iio/proximity/irsd200.c | 3 +-- drivers/iio/temperature/tmp006.c | 4 +--- 30 files changed, 34 insertions(+), 85 deletions(-)
Jonathan mentioned recently that he would like to get away from using memset() to zero-initialize stack memory in the IIO subsystem. And we have it on good authority that initializing a struct or array with = { } is the preferred way to do this in the kernel [1]. So here is a series to take care of that. [1]: https://lore.kernel.org/linux-iio/202505090942.48EBF01B@keescook/ --- David Lechner (28): iio: accel: adxl372: use = { } instead of memset() iio: accel: msa311: use = { } instead of memset() iio: adc: dln2-adc: use = { } instead of memset() iio: adc: mt6360-adc: use = { } instead of memset() iio: adc: rockchip_saradc: use = { } instead of memset() iio: adc: rtq6056: use = { } instead of memset() iio: adc: stm32-adc: use = { } instead of memset() iio: adc: ti-ads1015: use = { } instead of memset() iio: adc: ti-ads1119: use = { } instead of memset() iio: adc: ti-lmp92064: use = { } instead of memset() iio: adc: ti-tsc2046: use = { } instead of memset() iio: chemical: scd4x: use = { } instead of memset() iio: chemical: scd30: use = { } instead of memset() iio: chemical: sunrise_co2: use = { } instead of memset() iio: dac: ad3552r: use = { } instead of memset() iio: imu: inv_icm42600: use = { } instead of memset() iio: imu: inv_mpu6050: use = { } instead of memset() iio: light: bh1745: use = { } instead of memset() iio: light: ltr501: use = { } instead of memset() iio: light: opt4060: use = { } instead of memset() iio: light: veml6030: use = { } instead of memset() iio: magnetometer: af8133j: use = { } instead of memset() iio: pressure: bmp280: use = { } instead of memset() iio: pressure: mpl3115: use = { } instead of memset() iio: pressure: mprls0025pa: use = { } instead of memset() iio: pressure: zpa2326: use = { } instead of memset() iio: proximity: irsd200: use = { } instead of memset() iio: temperature: tmp006: use = { } instead of memset() drivers/iio/accel/adxl372.c | 3 +-- drivers/iio/accel/msa311.c | 4 +--- drivers/iio/adc/dln2-adc.c | 4 +--- drivers/iio/adc/mt6360-adc.c | 3 +-- drivers/iio/adc/rockchip_saradc.c | 4 +--- drivers/iio/adc/rtq6056.c | 4 +--- drivers/iio/adc/stm32-adc.c | 3 +-- drivers/iio/adc/ti-ads1015.c | 4 +--- drivers/iio/adc/ti-ads1119.c | 4 +--- drivers/iio/adc/ti-lmp92064.c | 4 +--- drivers/iio/adc/ti-tsc2046.c | 3 +-- drivers/iio/chemical/scd30_core.c | 3 +-- drivers/iio/chemical/scd4x.c | 3 +-- drivers/iio/chemical/sunrise_co2.c | 6 ++---- drivers/iio/dac/ad3552r.c | 3 +-- drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 5 ++--- drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c | 5 ++--- drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 4 +--- drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 6 ++---- drivers/iio/light/bh1745.c | 4 +--- drivers/iio/light/ltr501.c | 4 +--- drivers/iio/light/opt4060.c | 4 +--- drivers/iio/light/veml6030.c | 4 +--- drivers/iio/magnetometer/af8133j.c | 4 +--- drivers/iio/pressure/bmp280-core.c | 5 +---- drivers/iio/pressure/mpl3115.c | 3 +-- drivers/iio/pressure/mprls0025pa_i2c.c | 5 +---- drivers/iio/pressure/zpa2326.c | 4 +--- drivers/iio/proximity/irsd200.c | 3 +-- drivers/iio/temperature/tmp006.c | 4 +--- 30 files changed, 34 insertions(+), 85 deletions(-) --- base-commit: 4c6073fec2fee4827fa0dd8a4ab4e6f7bbc05ee6 change-id: 20250611-iio-zero-init-stack-with-instead-of-memset-0d12d41a7ecb Best regards, -- David Lechner <dlechner@baylibre.com>
Hi! > Jonathan mentioned recently that he would like to get away from using > memset() to zero-initialize stack memory in the IIO subsystem. And we > have it on good authority that initializing a struct or array with = { } > is the preferred way to do this in the kernel [1]. So here is a series > to take care of that. 1) Is it worth the churn? 2) Will this fail to initialize padding with some obscure compiler? 3) Why do you believe that {} is the preffered way? All we have is Kees' email that explains that = {} maybe works in configs he tested. BR, Pavel > [1]: > https://lore.kernel.org/linux-iio/202505090942.48EBF01B@keescook/ > --- > David Lechner (28): > iio: accel: adxl372: use = { } instead of memset() > iio: accel: msa311: use = { } instead of memset() > iio: adc: dln2-adc: use = { } instead of memset() > iio: adc: mt6360-adc: use = { } instead of memset() > iio: adc: rockchip_saradc: use = { } instead of memset() > iio: adc: rtq6056: use = { } instead of memset() > iio: adc: stm32-adc: use = { } instead of memset() > iio: adc: ti-ads1015: use = { } instead of memset() > iio: adc: ti-ads1119: use = { } instead of memset() > iio: adc: ti-lmp92064: use = { } instead of memset() > iio: adc: ti-tsc2046: use = { } instead of memset() > iio: chemical: scd4x: use = { } instead of memset() > iio: chemical: scd30: use = { } instead of memset() > iio: chemical: sunrise_co2: use = { } instead of memset() > iio: dac: ad3552r: use = { } instead of memset() > iio: imu: inv_icm42600: use = { } instead of memset() > iio: imu: inv_mpu6050: use = { } instead of memset() > iio: light: bh1745: use = { } instead of memset() > iio: light: ltr501: use = { } instead of memset() > iio: light: opt4060: use = { } instead of memset() > iio: light: veml6030: use = { } instead of memset() > iio: magnetometer: af8133j: use = { } instead of memset() > iio: pressure: bmp280: use = { } instead of memset() > iio: pressure: mpl3115: use = { } instead of memset() > iio: pressure: mprls0025pa: use = { } instead of memset() > iio: pressure: zpa2326: use = { } instead of memset() > iio: proximity: irsd200: use = { } instead of memset() > iio: temperature: tmp006: use = { } instead of memset() > > drivers/iio/accel/adxl372.c | 3 +-- > drivers/iio/accel/msa311.c | 4 +--- > drivers/iio/adc/dln2-adc.c | 4 +--- > drivers/iio/adc/mt6360-adc.c | 3 +-- > drivers/iio/adc/rockchip_saradc.c | 4 +--- > drivers/iio/adc/rtq6056.c | 4 +--- > drivers/iio/adc/stm32-adc.c | 3 +-- > drivers/iio/adc/ti-ads1015.c | 4 +--- > drivers/iio/adc/ti-ads1119.c | 4 +--- > drivers/iio/adc/ti-lmp92064.c | 4 +--- > drivers/iio/adc/ti-tsc2046.c | 3 +-- > drivers/iio/chemical/scd30_core.c | 3 +-- > drivers/iio/chemical/scd4x.c | 3 +-- > drivers/iio/chemical/sunrise_co2.c | 6 ++---- > drivers/iio/dac/ad3552r.c | 3 +-- > drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 5 ++--- > drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c | 5 ++--- > drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 4 +--- > drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 6 ++---- > drivers/iio/light/bh1745.c | 4 +--- > drivers/iio/light/ltr501.c | 4 +--- > drivers/iio/light/opt4060.c | 4 +--- > drivers/iio/light/veml6030.c | 4 +--- > drivers/iio/magnetometer/af8133j.c | 4 +--- > drivers/iio/pressure/bmp280-core.c | 5 +---- > drivers/iio/pressure/mpl3115.c | 3 +-- > drivers/iio/pressure/mprls0025pa_i2c.c | 5 +---- > drivers/iio/pressure/zpa2326.c | 4 +--- > drivers/iio/proximity/irsd200.c | 3 +-- > drivers/iio/temperature/tmp006.c | 4 +--- > 30 files changed, 34 insertions(+), 85 deletions(-) > --- > base-commit: 4c6073fec2fee4827fa0dd8a4ab4e6f7bbc05ee6 > change-id: 20250611-iio-zero-init-stack-with-instead-of-memset-0d12d41a7ecb > > Best regards, -- I don't work for Nazis and criminals, and neither should you. Boycott Putin, Trump, and Musk!
Hello, I thought I'd chime in as someone uninvolved because this seemed interesting. On Thursday, 12 June 2025 11:17:52 Central European Summer Time Pavel Machek wrote: > Hi! > > > Jonathan mentioned recently that he would like to get away from using > > memset() to zero-initialize stack memory in the IIO subsystem. And we > > have it on good authority that initializing a struct or array with = { } > > is the preferred way to do this in the kernel [1]. So here is a series > > to take care of that. > > 1) Is it worth the churn? > > 2) Will this fail to initialize padding with some obscure compiler? as of right now, the only two C compilers that are supported are GCC >= 8.1, and Clang >= 13.0.1. If anyone even manages to get the kernel to finish a build with something else, I think the compiler not implementing the C standard correctly is the least of their worries. My bigger worry is that = { } is only guaranteed to be as correct as memset on C23, and the kernel's standard right now is C11. For that reason alone, I don't think memset should be moved away from for now, unless someone can verify that every GCC release >= 8.1 and every Clang release >= 13.0.1 does the right thing here regardless. > > 3) Why do you believe that {} is the preffered way? All we have is > Kees' email that explains that = {} maybe works in configs he tested. = { } is guaranteed to work in C23, as per the standard, but again we're not on C23. The reason to prefer this is likely that it's easier for static analysis to see the struct as initialised, but that's me making assumptions here. A more human-centric argument is that once we're on a C standards version where = { } is guaranteed to be correct, then = { } is much more obviously correct to a reader than a memset with a value and a size somewhere later in the code. This argument is evident from the number of patches in this series where the memset and the declaration are not in the same hunk. That's the kind of stuff that keeps me awake at night, sweating profusely. Kind regards, Nicolas Frattaroli > > BR, > Pavel > > > [1]: > > https://lore.kernel.org/linux-iio/202505090942.48EBF01B@keescook/ > > > > > --- > > David Lechner (28): > > iio: accel: adxl372: use = { } instead of memset() > > iio: accel: msa311: use = { } instead of memset() > > iio: adc: dln2-adc: use = { } instead of memset() > > iio: adc: mt6360-adc: use = { } instead of memset() > > iio: adc: rockchip_saradc: use = { } instead of memset() > > iio: adc: rtq6056: use = { } instead of memset() > > iio: adc: stm32-adc: use = { } instead of memset() > > iio: adc: ti-ads1015: use = { } instead of memset() > > iio: adc: ti-ads1119: use = { } instead of memset() > > iio: adc: ti-lmp92064: use = { } instead of memset() > > iio: adc: ti-tsc2046: use = { } instead of memset() > > iio: chemical: scd4x: use = { } instead of memset() > > iio: chemical: scd30: use = { } instead of memset() > > iio: chemical: sunrise_co2: use = { } instead of memset() > > iio: dac: ad3552r: use = { } instead of memset() > > iio: imu: inv_icm42600: use = { } instead of memset() > > iio: imu: inv_mpu6050: use = { } instead of memset() > > iio: light: bh1745: use = { } instead of memset() > > iio: light: ltr501: use = { } instead of memset() > > iio: light: opt4060: use = { } instead of memset() > > iio: light: veml6030: use = { } instead of memset() > > iio: magnetometer: af8133j: use = { } instead of memset() > > iio: pressure: bmp280: use = { } instead of memset() > > iio: pressure: mpl3115: use = { } instead of memset() > > iio: pressure: mprls0025pa: use = { } instead of memset() > > iio: pressure: zpa2326: use = { } instead of memset() > > iio: proximity: irsd200: use = { } instead of memset() > > iio: temperature: tmp006: use = { } instead of memset() > > > > drivers/iio/accel/adxl372.c | 3 +-- > > drivers/iio/accel/msa311.c | 4 +--- > > drivers/iio/adc/dln2-adc.c | 4 +--- > > drivers/iio/adc/mt6360-adc.c | 3 +-- > > drivers/iio/adc/rockchip_saradc.c | 4 +--- > > drivers/iio/adc/rtq6056.c | 4 +--- > > drivers/iio/adc/stm32-adc.c | 3 +-- > > drivers/iio/adc/ti-ads1015.c | 4 +--- > > drivers/iio/adc/ti-ads1119.c | 4 +--- > > drivers/iio/adc/ti-lmp92064.c | 4 +--- > > drivers/iio/adc/ti-tsc2046.c | 3 +-- > > drivers/iio/chemical/scd30_core.c | 3 +-- > > drivers/iio/chemical/scd4x.c | 3 +-- > > drivers/iio/chemical/sunrise_co2.c | 6 ++---- > > drivers/iio/dac/ad3552r.c | 3 +-- > > drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 5 ++--- > > drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c | 5 ++--- > > drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 4 +--- > > drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 6 ++---- > > drivers/iio/light/bh1745.c | 4 +--- > > drivers/iio/light/ltr501.c | 4 +--- > > drivers/iio/light/opt4060.c | 4 +--- > > drivers/iio/light/veml6030.c | 4 +--- > > drivers/iio/magnetometer/af8133j.c | 4 +--- > > drivers/iio/pressure/bmp280-core.c | 5 +---- > > drivers/iio/pressure/mpl3115.c | 3 +-- > > drivers/iio/pressure/mprls0025pa_i2c.c | 5 +---- > > drivers/iio/pressure/zpa2326.c | 4 +--- > > drivers/iio/proximity/irsd200.c | 3 +-- > > drivers/iio/temperature/tmp006.c | 4 +--- > > 30 files changed, 34 insertions(+), 85 deletions(-) > > --- > > base-commit: 4c6073fec2fee4827fa0dd8a4ab4e6f7bbc05ee6 > > change-id: 20250611-iio-zero-init-stack-with-instead-of-memset-0d12d41a7ecb > > > > Best regards, > >
Hi1 > I thought I'd chime in as someone uninvolved because this seemed > interesting. > > On Thursday, 12 June 2025 11:17:52 Central European Summer Time Pavel Machek wrote: > > Hi! > > > > > Jonathan mentioned recently that he would like to get away from using > > > memset() to zero-initialize stack memory in the IIO subsystem. And we > > > have it on good authority that initializing a struct or array with = { } > > > is the preferred way to do this in the kernel [1]. So here is a series > > > to take care of that. > > > > 1) Is it worth the churn? > > > > 2) Will this fail to initialize padding with some obscure compiler? > > as of right now, the only two C compilers that are supported are > GCC >= 8.1, and Clang >= 13.0.1. If anyone even manages to get the > kernel Well... I'm pretty sure parts of this would make it into -stable as a dependency, or because AUTOSEL decides it is a bugfix. So.. GNU C 4.9 gcc --version Clang/LLVM (optional) 10.0.1 clang --version :-). Best regards, Pavel -- I don't work for Nazis and criminals, and neither should you. Boycott Putin, Trump, and Musk!
On Thu, Jun 12, 2025 at 08:54:07PM +0200, Pavel Machek wrote: > > On Thursday, 12 June 2025 11:17:52 Central European Summer Time Pavel Machek wrote: > > > > > > > Jonathan mentioned recently that he would like to get away from using > > > > memset() to zero-initialize stack memory in the IIO subsystem. And we > > > > have it on good authority that initializing a struct or array with = { } > > > > is the preferred way to do this in the kernel [1]. So here is a series > > > > to take care of that. > > > > > > 1) Is it worth the churn? > > > > > > 2) Will this fail to initialize padding with some obscure compiler? > > > > as of right now, the only two C compilers that are supported are > > GCC >= 8.1, and Clang >= 13.0.1. If anyone even manages to get the > > kernel > > Well... I'm pretty sure parts of this would make it into -stable as a > dependency, or because AUTOSEL decides it is a bugfix. So.. > > GNU C 4.9 gcc --version > Clang/LLVM (optional) 10.0.1 clang --version Even though, what the kernel versions are you referring to? I am sure there plenty of cases with {} there. -- With Best Regards, Andy Shevchenko
On Thu 2025-06-12 22:10:07, Andy Shevchenko wrote: > On Thu, Jun 12, 2025 at 08:54:07PM +0200, Pavel Machek wrote: > > > On Thursday, 12 June 2025 11:17:52 Central European Summer Time Pavel Machek wrote: > > > > > > > > > Jonathan mentioned recently that he would like to get away from using > > > > > memset() to zero-initialize stack memory in the IIO subsystem. And we > > > > > have it on good authority that initializing a struct or array with = { } > > > > > is the preferred way to do this in the kernel [1]. So here is a series > > > > > to take care of that. > > > > > > > > 1) Is it worth the churn? > > > > > > > > 2) Will this fail to initialize padding with some obscure compiler? > > > > > > as of right now, the only two C compilers that are supported are > > > GCC >= 8.1, and Clang >= 13.0.1. If anyone even manages to get the > > > kernel > > > > Well... I'm pretty sure parts of this would make it into -stable as a > > dependency, or because AUTOSEL decides it is a bugfix. So.. > > > > GNU C 4.9 gcc --version > > Clang/LLVM (optional) 10.0.1 clang --version > > Even though, what the kernel versions are you referring to? I am sure there > plenty of cases with {} there. 5.10, for example. I'm sure they are, uninitialized padding is a security hole, but rather hard to detect if they are not specifically looking. BR, Pavel -- I don't work for Nazis and criminals, and neither should you. Boycott Putin, Trump, and Musk!
On Sat, 14 Jun 2025 08:47:25 +0200 Pavel Machek <pavel@ucw.cz> wrote: > On Thu 2025-06-12 22:10:07, Andy Shevchenko wrote: > > On Thu, Jun 12, 2025 at 08:54:07PM +0200, Pavel Machek wrote: > > > > On Thursday, 12 June 2025 11:17:52 Central European Summer Time Pavel Machek wrote: > > > > > > > > > > > Jonathan mentioned recently that he would like to get away from using > > > > > > memset() to zero-initialize stack memory in the IIO subsystem. And we > > > > > > have it on good authority that initializing a struct or array with = { } > > > > > > is the preferred way to do this in the kernel [1]. So here is a series > > > > > > to take care of that. > > > > > > > > > > 1) Is it worth the churn? > > > > > > > > > > 2) Will this fail to initialize padding with some obscure compiler? > > > > > > > > as of right now, the only two C compilers that are supported are > > > > GCC >= 8.1, and Clang >= 13.0.1. If anyone even manages to get the > > > > kernel > > > > > > Well... I'm pretty sure parts of this would make it into -stable as a > > > dependency, or because AUTOSEL decides it is a bugfix. So.. > > > > > > GNU C 4.9 gcc --version > > > Clang/LLVM (optional) 10.0.1 clang --version > > > > Even though, what the kernel versions are you referring to? I am sure there > > plenty of cases with {} there. > > 5.10, for example. I'm sure they are, uninitialized padding is a > security hole, but rather hard to detect if they are not specifically > looking. The stack kunit test is there back to 5.0-rc4 50ceaa95ea09 ("lib: Introduce test_stackinit module") So I think we should be pretty well defended against issues. Hence I plan to pick this up curently. Thanks all for inputs on this. Fun corners of the C spec vs implementations! Jonathan > > BR, > Pavel
On Sat, 14 Jun 2025 13:18:44 +0100 Jonathan Cameron <jic23@kernel.org> wrote: > On Sat, 14 Jun 2025 08:47:25 +0200 > Pavel Machek <pavel@ucw.cz> wrote: > > > On Thu 2025-06-12 22:10:07, Andy Shevchenko wrote: > > > On Thu, Jun 12, 2025 at 08:54:07PM +0200, Pavel Machek wrote: > > > > > On Thursday, 12 June 2025 11:17:52 Central European Summer Time Pavel Machek wrote: > > > > > > > > > > > > > Jonathan mentioned recently that he would like to get away from using > > > > > > > memset() to zero-initialize stack memory in the IIO subsystem. And we > > > > > > > have it on good authority that initializing a struct or array with = { } > > > > > > > is the preferred way to do this in the kernel [1]. So here is a series > > > > > > > to take care of that. > > > > > > > > > > > > 1) Is it worth the churn? > > > > > > > > > > > > 2) Will this fail to initialize padding with some obscure compiler? > > > > > > > > > > as of right now, the only two C compilers that are supported are > > > > > GCC >= 8.1, and Clang >= 13.0.1. If anyone even manages to get the > > > > > kernel > > > > > > > > Well... I'm pretty sure parts of this would make it into -stable as a > > > > dependency, or because AUTOSEL decides it is a bugfix. So.. > > > > > > > > GNU C 4.9 gcc --version > > > > Clang/LLVM (optional) 10.0.1 clang --version > > > > > > Even though, what the kernel versions are you referring to? I am sure there > > > plenty of cases with {} there. > > > > 5.10, for example. I'm sure they are, uninitialized padding is a > > security hole, but rather hard to detect if they are not specifically > > looking. > > The stack kunit test is there back to 5.0-rc4 > 50ceaa95ea09 ("lib: Introduce test_stackinit module") > > So I think we should be pretty well defended against issues. > > Hence I plan to pick this up curently. > > Thanks all for inputs on this. > > Fun corners of the C spec vs implementations! > > Jonathan > I want to give this some testing exposure from 0-day etc in case we missed any build related issues so I've queued it up on my testing branch. I can still pick up tags / rebase etc for now. Thanks, Jonathan > > > > BR, > > Pavel > >
On Thu, Jun 12, 2025 at 3:12 PM Nicolas Frattaroli <nicolas.frattaroli@collabora.com> wrote: > I thought I'd chime in as someone uninvolved because this seemed > interesting. Welcome! Other opinions on such a topic are always appreciated. > On Thursday, 12 June 2025 11:17:52 Central European Summer Time Pavel Machek wrote: > > > > > Jonathan mentioned recently that he would like to get away from using > > > memset() to zero-initialize stack memory in the IIO subsystem. And we > > > have it on good authority that initializing a struct or array with = { } > > > is the preferred way to do this in the kernel [1]. So here is a series > > > to take care of that. > > > > 1) Is it worth the churn? > > > > 2) Will this fail to initialize padding with some obscure compiler? > > as of right now, the only two C compilers that are supported are > GCC >= 8.1, and Clang >= 13.0.1. If anyone even manages to get the kernel > to finish a build with something else, I think the compiler not > implementing the C standard correctly is the least of their worries. > > My bigger worry is that = { } is only guaranteed to be as correct as > memset on C23, and the kernel's standard right now is C11. For that > reason alone, I don't think memset should be moved away from for now, > unless someone can verify that every GCC release >= 8.1 and every > Clang release >= 13.0.1 does the right thing here regardless. > > > > > 3) Why do you believe that {} is the preffered way? All we have is > > Kees' email that explains that = {} maybe works in configs he tested. > > = { } is guaranteed to work in C23, as per the standard, but again we're > not on C23. > > The reason to prefer this is likely that it's easier for static analysis > to see the struct as initialised, but that's me making assumptions here. > > A more human-centric argument is that once we're on a C standards version > where = { } is guaranteed to be correct, then = { } is much more obviously > correct to a reader than a memset with a value and a size somewhere later > in the code. This argument is evident from the number of patches in this > series where the memset and the declaration are not in the same hunk. > That's the kind of stuff that keeps me awake at night, sweating profusely. While all you said seems true and I agree with, the pedantism here is not needed as in the Linux kernel we have {} used for ages in tons of code and if something went wrong with that we should have had bug reports already. Are you aware of such? Personally I haven't heard even one related to this. But if you know, I am really more than interested to read about (please, give pointers to such a discussion). -- With Best Regards, Andy Shevchenko
On Thu, 12 Jun 2025 11:17:52 +0200 Pavel Machek <pavel@ucw.cz> wrote: > Hi! > > > Jonathan mentioned recently that he would like to get away from using > > memset() to zero-initialize stack memory in the IIO subsystem. And we > > have it on good authority that initializing a struct or array with = { } > > is the preferred way to do this in the kernel [1]. So here is a series > > to take care of that. > > 1) Is it worth the churn? > > 2) Will this fail to initialize padding with some obscure compiler? > > 3) Why do you believe that {} is the preffered way? All we have is > Kees' email that explains that = {} maybe works in configs he tested. > Pavel, I think main thing that matters in Kees email is there is a self test that should fire if a compiler ever does this wrong. Using this syntax is definitely not a 'kernel wide' preference yet but I do prefer to make some changes like this in IIO just because it reduces the amount of code that smells different when reviewing. Given how many drivers we now have, sadly people pick different ones to cut and paste from so we get a lot of new drivers that look like how we preferred to do things 10 years ago :( However it is a fair bit of churn. hmm. Let's let this sit for a little while and see if other view points come in. Thanks Jonathan > BR, > Pavel > > > [1]: > > https://lore.kernel.org/linux-iio/202505090942.48EBF01B@keescook/ > > > > > --- > > David Lechner (28): > > iio: accel: adxl372: use = { } instead of memset() > > iio: accel: msa311: use = { } instead of memset() > > iio: adc: dln2-adc: use = { } instead of memset() > > iio: adc: mt6360-adc: use = { } instead of memset() > > iio: adc: rockchip_saradc: use = { } instead of memset() > > iio: adc: rtq6056: use = { } instead of memset() > > iio: adc: stm32-adc: use = { } instead of memset() > > iio: adc: ti-ads1015: use = { } instead of memset() > > iio: adc: ti-ads1119: use = { } instead of memset() > > iio: adc: ti-lmp92064: use = { } instead of memset() > > iio: adc: ti-tsc2046: use = { } instead of memset() > > iio: chemical: scd4x: use = { } instead of memset() > > iio: chemical: scd30: use = { } instead of memset() > > iio: chemical: sunrise_co2: use = { } instead of memset() > > iio: dac: ad3552r: use = { } instead of memset() > > iio: imu: inv_icm42600: use = { } instead of memset() > > iio: imu: inv_mpu6050: use = { } instead of memset() > > iio: light: bh1745: use = { } instead of memset() > > iio: light: ltr501: use = { } instead of memset() > > iio: light: opt4060: use = { } instead of memset() > > iio: light: veml6030: use = { } instead of memset() > > iio: magnetometer: af8133j: use = { } instead of memset() > > iio: pressure: bmp280: use = { } instead of memset() > > iio: pressure: mpl3115: use = { } instead of memset() > > iio: pressure: mprls0025pa: use = { } instead of memset() > > iio: pressure: zpa2326: use = { } instead of memset() > > iio: proximity: irsd200: use = { } instead of memset() > > iio: temperature: tmp006: use = { } instead of memset() > > > > drivers/iio/accel/adxl372.c | 3 +-- > > drivers/iio/accel/msa311.c | 4 +--- > > drivers/iio/adc/dln2-adc.c | 4 +--- > > drivers/iio/adc/mt6360-adc.c | 3 +-- > > drivers/iio/adc/rockchip_saradc.c | 4 +--- > > drivers/iio/adc/rtq6056.c | 4 +--- > > drivers/iio/adc/stm32-adc.c | 3 +-- > > drivers/iio/adc/ti-ads1015.c | 4 +--- > > drivers/iio/adc/ti-ads1119.c | 4 +--- > > drivers/iio/adc/ti-lmp92064.c | 4 +--- > > drivers/iio/adc/ti-tsc2046.c | 3 +-- > > drivers/iio/chemical/scd30_core.c | 3 +-- > > drivers/iio/chemical/scd4x.c | 3 +-- > > drivers/iio/chemical/sunrise_co2.c | 6 ++---- > > drivers/iio/dac/ad3552r.c | 3 +-- > > drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 5 ++--- > > drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c | 5 ++--- > > drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 4 +--- > > drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 6 ++---- > > drivers/iio/light/bh1745.c | 4 +--- > > drivers/iio/light/ltr501.c | 4 +--- > > drivers/iio/light/opt4060.c | 4 +--- > > drivers/iio/light/veml6030.c | 4 +--- > > drivers/iio/magnetometer/af8133j.c | 4 +--- > > drivers/iio/pressure/bmp280-core.c | 5 +---- > > drivers/iio/pressure/mpl3115.c | 3 +-- > > drivers/iio/pressure/mprls0025pa_i2c.c | 5 +---- > > drivers/iio/pressure/zpa2326.c | 4 +--- > > drivers/iio/proximity/irsd200.c | 3 +-- > > drivers/iio/temperature/tmp006.c | 4 +--- > > 30 files changed, 34 insertions(+), 85 deletions(-) > > --- > > base-commit: 4c6073fec2fee4827fa0dd8a4ab4e6f7bbc05ee6 > > change-id: 20250611-iio-zero-init-stack-with-instead-of-memset-0d12d41a7ecb > > > > Best regards, >
On Wed, Jun 11, 2025 at 05:38:52PM -0500, David Lechner wrote: > Jonathan mentioned recently that he would like to get away from using > memset() to zero-initialize stack memory in the IIO subsystem. And we > have it on good authority that initializing a struct or array with = { } > is the preferred way to do this in the kernel [1]. So here is a series > to take care of that. I believe we may do that independently of the compilers that can leave garbage in the padding. In case it happens, it will be good adventure to fix the bugs in the code, that for some reason take padding(s) into account for the real values. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> -- With Best Regards, Andy Shevchenko
On Wed, 2025-06-11 at 17:38 -0500, David Lechner wrote: > Jonathan mentioned recently that he would like to get away from using > memset() to zero-initialize stack memory in the IIO subsystem. And we > have it on good authority that initializing a struct or array with = { } > is the preferred way to do this in the kernel [1]. So here is a series > to take care of that. > > [1]: https://lore.kernel.org/linux-iio/202505090942.48EBF01B@keescook/ > > --- Reviewed-by: Nuno Sá <nuno.sa@analog.com> > David Lechner (28): > iio: accel: adxl372: use = { } instead of memset() > iio: accel: msa311: use = { } instead of memset() > iio: adc: dln2-adc: use = { } instead of memset() > iio: adc: mt6360-adc: use = { } instead of memset() > iio: adc: rockchip_saradc: use = { } instead of memset() > iio: adc: rtq6056: use = { } instead of memset() > iio: adc: stm32-adc: use = { } instead of memset() > iio: adc: ti-ads1015: use = { } instead of memset() > iio: adc: ti-ads1119: use = { } instead of memset() > iio: adc: ti-lmp92064: use = { } instead of memset() > iio: adc: ti-tsc2046: use = { } instead of memset() > iio: chemical: scd4x: use = { } instead of memset() > iio: chemical: scd30: use = { } instead of memset() > iio: chemical: sunrise_co2: use = { } instead of memset() > iio: dac: ad3552r: use = { } instead of memset() > iio: imu: inv_icm42600: use = { } instead of memset() > iio: imu: inv_mpu6050: use = { } instead of memset() > iio: light: bh1745: use = { } instead of memset() > iio: light: ltr501: use = { } instead of memset() > iio: light: opt4060: use = { } instead of memset() > iio: light: veml6030: use = { } instead of memset() > iio: magnetometer: af8133j: use = { } instead of memset() > iio: pressure: bmp280: use = { } instead of memset() > iio: pressure: mpl3115: use = { } instead of memset() > iio: pressure: mprls0025pa: use = { } instead of memset() > iio: pressure: zpa2326: use = { } instead of memset() > iio: proximity: irsd200: use = { } instead of memset() > iio: temperature: tmp006: use = { } instead of memset() > > drivers/iio/accel/adxl372.c | 3 +-- > drivers/iio/accel/msa311.c | 4 +--- > drivers/iio/adc/dln2-adc.c | 4 +--- > drivers/iio/adc/mt6360-adc.c | 3 +-- > drivers/iio/adc/rockchip_saradc.c | 4 +--- > drivers/iio/adc/rtq6056.c | 4 +--- > drivers/iio/adc/stm32-adc.c | 3 +-- > drivers/iio/adc/ti-ads1015.c | 4 +--- > drivers/iio/adc/ti-ads1119.c | 4 +--- > drivers/iio/adc/ti-lmp92064.c | 4 +--- > drivers/iio/adc/ti-tsc2046.c | 3 +-- > drivers/iio/chemical/scd30_core.c | 3 +-- > drivers/iio/chemical/scd4x.c | 3 +-- > drivers/iio/chemical/sunrise_co2.c | 6 ++---- > drivers/iio/dac/ad3552r.c | 3 +-- > drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 5 ++--- > drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c | 5 ++--- > drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 4 +--- > drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 6 ++---- > drivers/iio/light/bh1745.c | 4 +--- > drivers/iio/light/ltr501.c | 4 +--- > drivers/iio/light/opt4060.c | 4 +--- > drivers/iio/light/veml6030.c | 4 +--- > drivers/iio/magnetometer/af8133j.c | 4 +--- > drivers/iio/pressure/bmp280-core.c | 5 +---- > drivers/iio/pressure/mpl3115.c | 3 +-- > drivers/iio/pressure/mprls0025pa_i2c.c | 5 +---- > drivers/iio/pressure/zpa2326.c | 4 +--- > drivers/iio/proximity/irsd200.c | 3 +-- > drivers/iio/temperature/tmp006.c | 4 +--- > 30 files changed, 34 insertions(+), 85 deletions(-) > --- > base-commit: 4c6073fec2fee4827fa0dd8a4ab4e6f7bbc05ee6 > change-id: 20250611-iio-zero-init-stack-with-instead-of-memset-0d12d41a7ecb > > Best regards,
© 2016 - 2025 Red Hat, Inc.