3:34 a.m.
On Mon, Aug 5, 2019 at 10:06 AM Jeff Moyer <jmoyer(a)redhat.com> wrote:
Dan Williams <dan.j.williams(a)intel.com> writes:
> gcc 9.1.1 emits a slew of warnings for many of the command field
> accesses. I.e. warnings of the form:
>
> libndctl.c:2586:21: warning: taking address of packed member of ‘struct
nd_cmd_get_config_data_hdr’ may result in an unaligned pointer value
[-Waddress-of-packed-member]
> 2586 | cmd->iter.offset = &cmd->get_data->in_offset;
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
>
> Suppress these as fixing the warning would defeat the abstraction of being able
> to have common code that operates on commands with common fields at different
> offsets in the payload.
As I understand it, taking a pointer to this potentially unaligned
member can result in bus errors (or worse, accessing wrong data) on
architectures that don't support unaligned accesses. I'd be a whole lot
happier with this changelog if it mentioned that you had considered what
the warning actually meant, and decided it didn't matter for the
architectures you want to support.
True, it was a fleeting thought, but not something I considered
fleshing out... I'll send a revision.
x86 is, of course, safe. I believe aarch64 is, as well. I didn't look
into others.
Keep in mind that this code is for interfacing with the ACPI DSM path.
If an unaligned-incapable architecture defined an NVDIMM command set
it is highly unlikely it would be ACPI based, or pick these
problematic command formats. I can add these notes to the changelog,
but the unfortunate definition of these payloads that require __packed
is something I hope other architectures avoid.