Messages in this thread | | | Subject | Re: [PATCH v2] x86/boot: save fields explicitly, zero out everything else | From | John Hubbard <> | Date | Wed, 7 Aug 2019 12:43:08 -0700 |
| |
On 8/7/19 4:41 AM, David Laight wrote: > From: john.hubbard@gmail.com >> Sent: 31 July 2019 06:46 ... >> if (boot_params->sentinel) { >> - /* fields in boot_params are left uninitialized, clear them */ >> - boot_params->acpi_rsdp_addr = 0; >> - memset(&boot_params->ext_ramdisk_image, 0, >> - (char *)&boot_params->efi_info - >> - (char *)&boot_params->ext_ramdisk_image); >> - memset(&boot_params->kbd_status, 0, >> - (char *)&boot_params->hdr - >> - (char *)&boot_params->kbd_status); >> - memset(&boot_params->_pad7[0], 0, >> - (char *)&boot_params->edd_mbr_sig_buffer[0] - >> - (char *)&boot_params->_pad7[0]); >> - memset(&boot_params->_pad8[0], 0, >> - (char *)&boot_params->eddbuf[0] - >> - (char *)&boot_params->_pad8[0]); >> - memset(&boot_params->_pad9[0], 0, sizeof(boot_params->_pad9)); > ... > > How about replacing the above first using: > #define zero_struct_fields(ptr, from, to) memset(&ptr->from, 0, (char *)&ptr->to - (char *)&ptr->from) > zero_struct_fields(boot_params, ext_ramdisk_image, efi_info); > ... > Which is absolutely identical to the original code. > > The replacing the define with: > #define so(s, m) offsetof(typeof(*s), m) > #define zero_struct_fields(ptr, from, to) memset((char *)ptr + so(ptr, from), 0, so(ptr, to) - so(ptr, from)) > which gcc probably doesn't complain about, but should generate identical code again. > There might be an existing define for so(). >
Hi David,
There was discussion about that [1], but preference ending up being to flip this around, in order to more closely match the original intent of this function (zero out everything except for certain carefully selected fields), and to therefore be more likely to keep working if fields are added.
[1] https://lore.kernel.org/lkml/alpine.DEB.2.21.1907252358240.1791@nanos.tec.linutronix.de/
thanks, -- John Hubbard NVIDIA
| |