On Mon, Nov 12, 2018 at 02:42:16PM +0000, Peter Maydell wrote:
> Since nobody responded to my ping of a week ago I propose to just
> apply this to master...
>
> thanks
> -- PMM
Sorry. My LPC talk proposal suddenly got accepted and
I was scrambling to get ready.
Please feel free to apply this for now:
Reviewed-by: Michael S. Tsirkin <m...@redhat.com>
Generally I think we need to rethink our approach to endian-ness. I
think we want to tag fields with specific endian-ness and use static
checkers to verify it. That is how Linux does it.
In particular this will mean no swapping bytes in place.
But that's a subject for another day.
> On 5 November 2018 at 14:40, Peter Maydell <peter.mayd...@linaro.org> wrote:
> > Ping? This patch got reviewed but does not seem to have
> > made it into anybody's tree.
> >
> > thanks
> > -- PMM
> >
> > On 16 October 2018 at 18:52, Peter Maydell <peter.mayd...@linaro.org> wrote:
> >> Taking the address of a field in a packed struct is a bad idea, because
> >> it might not be actually aligned enough for that pointer type (and
> >> thus cause a crash on dereference on some host architectures). Newer
> >> versions of clang warn about this. Avoid the bug by not using the
> >> "modify in place" byte swapping functions.
> >>
> >> Patch produced with scripts/coccinelle/inplace-byteswaps.cocci.
> >>
> >> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> >> ---
> >> Automatically generated patch, tested with "make check" only.
> >>
> >> hw/acpi/nvdimm.c | 16 ++++++++--------
> >> 1 file changed, 8 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> >> index 27eeb6609f5..e53b2cb6819 100644
> >> --- a/hw/acpi/nvdimm.c
> >> +++ b/hw/acpi/nvdimm.c
> >> @@ -581,7 +581,7 @@ static void nvdimm_dsm_func_read_fit(AcpiNVDIMMState
> >> *state, NvdimmDsmIn *in,
> >> int size;
> >>
> >> read_fit = (NvdimmFuncReadFITIn *)in->arg3;
> >> - le32_to_cpus(&read_fit->offset);
> >> + read_fit->offset = le32_to_cpu(read_fit->offset);
> >>
> >> fit = fit_buf->fit;
> >>
> >> @@ -742,8 +742,8 @@ static void nvdimm_dsm_get_label_data(NVDIMMDevice
> >> *nvdimm, NvdimmDsmIn *in,
> >> int size;
> >>
> >> get_label_data = (NvdimmFuncGetLabelDataIn *)in->arg3;
> >> - le32_to_cpus(&get_label_data->offset);
> >> - le32_to_cpus(&get_label_data->length);
> >> + get_label_data->offset = le32_to_cpu(get_label_data->offset);
> >> + get_label_data->length = le32_to_cpu(get_label_data->length);
> >>
> >> nvdimm_debug("Read Label Data: offset %#x length %#x.\n",
> >> get_label_data->offset, get_label_data->length);
> >> @@ -781,8 +781,8 @@ static void nvdimm_dsm_set_label_data(NVDIMMDevice
> >> *nvdimm, NvdimmDsmIn *in,
> >>
> >> set_label_data = (NvdimmFuncSetLabelDataIn *)in->arg3;
> >>
> >> - le32_to_cpus(&set_label_data->offset);
> >> - le32_to_cpus(&set_label_data->length);
> >> + set_label_data->offset = le32_to_cpu(set_label_data->offset);
> >> + set_label_data->length = le32_to_cpu(set_label_data->length);
> >>
> >> nvdimm_debug("Write Label Data: offset %#x length %#x.\n",
> >> set_label_data->offset, set_label_data->length);
> >> @@ -877,9 +877,9 @@ nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t
> >> val, unsigned size)
> >> in = g_new(NvdimmDsmIn, 1);
> >> cpu_physical_memory_read(dsm_mem_addr, in, sizeof(*in));
> >>
> >> - le32_to_cpus(&in->revision);
> >> - le32_to_cpus(&in->function);
> >> - le32_to_cpus(&in->handle);
> >> + in->revision = le32_to_cpu(in->revision);
> >> + in->function = le32_to_cpu(in->function);
> >> + in->handle = le32_to_cpu(in->handle);
> >>
> >> nvdimm_debug("Revision %#x Handler %#x Function %#x.\n", in->revision,
> >> in->handle, in->function);
> >> --
