Re: [PATCH v2] x86/boot: save fields explicitly, zero out everything else

[John Hubbard]

On 8/7/19 4:41 AM, David Laight wrote:
> From: john.hubbard@xxxxxxxxx
>> 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@xxxxxxxxxxxxxxxxxxxxxxx/

thanks,
-- 
John Hubbard
NVIDIA