From: Dan Williams <dan.j.williams@intel.com>
To: Jeff Moyer <jmoyer@redhat.com>
Cc: linux-nvdimm <linux-nvdimm@lists.01.org>
Subject: Re: [ndctl PATCH v3 1/8] ndctl/build: Suppress -Waddress-of-packed-member
Date: Mon, 5 Aug 2019 10:34:23 -0700 [thread overview]
Message-ID: <CAPcyv4jZVMFqd7G++vsOxCxPWp-bUUQ4KKAq0-oku0=hAE5zzA@mail.gmail.com> (raw)
In-Reply-To: <x491rxzr1rq.fsf@segfault.boston.devel.redhat.com>
On Mon, Aug 5, 2019 at 10:06 AM Jeff Moyer <jmoyer@redhat.com> wrote:
>
> Dan Williams <dan.j.williams@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.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
next prev parent reply other threads:[~2019-08-05 17:37 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-02 23:54 [ndctl PATCH v3 0/8] Improvements for namespace creation/interrogation Dan Williams
2019-08-02 23:54 ` [ndctl PATCH v3 1/8] ndctl/build: Suppress -Waddress-of-packed-member Dan Williams
2019-08-05 16:54 ` Jeff Moyer
2019-08-05 17:34 ` Dan Williams [this message]
2019-08-05 17:45 ` Jeff Moyer
2019-08-05 19:50 ` Dan Williams
2019-08-02 23:54 ` [ndctl PATCH v3 2/8] ndctl/dimm: Support small label reads/writes Dan Williams
2019-08-02 23:54 ` [ndctl PATCH v3 3/8] ndctl/dimm: Minimize data-transfer for init-labels Dan Williams
2019-08-02 23:54 ` [ndctl PATCH v3 4/8] ndctl/dimm: Add offset and size options to {read, write, zero}-labels Dan Williams
2019-08-02 23:54 ` [ndctl PATCH v3 5/8] ndctl/dimm: Limit read-labels with --index option Dan Williams
2019-08-02 23:54 ` [ndctl PATCH v3 6/8] ndctl/namespace: Minimize label data transfer for autolabel Dan Williams
2019-08-02 23:55 ` [ndctl PATCH v3 7/8] ndctl/namespace: Continue region search on 'missing seed' event Dan Williams
2019-08-02 23:55 ` [ndctl PATCH v3 8/8] ndctl/namespace: Report ENOSPC when regions are full Dan Williams
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAPcyv4jZVMFqd7G++vsOxCxPWp-bUUQ4KKAq0-oku0=hAE5zzA@mail.gmail.com' \
--to=dan.j.williams@intel.com \
--cc=jmoyer@redhat.com \
--cc=linux-nvdimm@lists.01.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).