On Tue, 25 Oct 2022, Starke, Daniel wrote: > > > + n1 = FIELD_GET(PN_N_FIELD_N1, le16_to_cpu(params->n_bits)); > > > > Should this be using get_unaligned...()? > > Is this really necessary if the structure is already __packed? I did not > receive any warning by the compiler. It would be arch dependent to begin with. But honestly, I'm not entirely certain here myself. Documentation/core-api/unaligned-memory-access.rst claims compiler would indeed do extra work to ensure access of unaligned member in a packed struct is handled ok. But then you call le16_to_cpu() for the member field which is full of cast magic so I'd be a bit hesitant to claim the knowledge about the unalignment is carried all the way down there through those casts. Other subtle detail is the reply side struct which is allocated from stack and with packed compiler is allowed (I don't know if it does that or not) to make the struct unaligned as well (so perhaps put_unaligned would be necessary there too if packed is retained). If you want my recommendation, I'd just remove the packed altogether from the struct because there seems to be no natural holes in it, use get_unaligned for the receive side, and add this build time check: static_assert(sizeof(struct gsm_dlci_param_bits) == 8); If lkp builds all its current archs fine with that static_assert(), I'd be pretty sure the struct that the unpacked struct is ok on all archs. Would it ever stop being true on any arch/compiler setting, the assert would catch it right away. -- i.
