Re: [Qemu-devel] [PATCH for-2.9] tests/virtio-9p-test: Don't call le*_to_cpus on fields of packed struct

[Greg Kurz]

On Mon, 27 Mar 2017 18:59:04 +0100
Peter Maydell <address@hidden> wrote:

> For a packed struct like 'P9Hdr' the fields within it may not be
> aligned as much as the natural alignment for their types.  This means
> it is not valid to pass the address of such a field to a function
> like le32_to_cpus() which operate on uint32_t* and assume alignment.
> Doing this results in a SIGBUS on hosts like SPARC which have strict
> alignment requirements.
> 
> Use ldl_le_p() instead, which is specified to correctly handle
> unaligned pointers.
> 
> Signed-off-by: Peter Maydell <address@hidden>
> ---

Reviewed-by: Greg Kurz <address@hidden>

I'll apply this to my tree and send a pull request tomorrow.

Cheers.

--
Greg

> Sadly gcc doesn't warn about this:
>  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628
> clang supposedly was fixed to do so:
>  https://bugs.llvm.org//show_bug.cgi?id=22821
> but I think that commit was reverted without the bug being
> reopened; at least my clang doesn't have that warning flag.
> ---
>  tests/virtio-9p-test.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
> index 43a1ad8..ad33d96 100644
> --- a/tests/virtio-9p-test.c
> +++ b/tests/virtio-9p-test.c
> @@ -256,8 +256,8 @@ static void v9fs_req_recv(P9Req *req, uint8_t id)
>          qvirtio_wait_queue_isr(v9p->dev, v9p->vq, 1000 * 1000);
>  
>          v9fs_memread(req, &hdr, 7);
> -        le32_to_cpus(&hdr.size);
> -        le16_to_cpus(&hdr.tag);
> +        hdr.size = ldl_le_p(&hdr.size);
> +        hdr.tag = lduw_le_p(&hdr.tag);
>          if (hdr.size >= 7) {
>              break;
>          }

pgpN2Lhjyo_ne.pgp
Description: OpenPGP digital signature