Declare packed struct ata_params as 2-byte-aligned

[rlibby]

Summary

This avoids gcc9 warning about unaligned access to the structure when
casting to uint16_t pointer type.

Test Plan

make CROSS_TOOLCHAIN=amd64-gcc9 buildworld

[imp]

It is actually properly aligned. Let's not do silly crap like this. Let's say it really is properly aligned.

[rlibby]

In D22888#501055, @imp wrote:

It is actually properly aligned. Let's not do silly crap like this. Let's say it really is properly aligned.

It is indeed properly aligned, and gcc doesn't know it. How would you prefer to silence the warning? I felt it was just as much effort to do it in the Makefile.

[rlibby]

A non-intrusive Makefile solution could be to lower camcontrol's WARNS to 3, either unconditionally or conditional on gcc.

For more background, this is to get gcc9 working. This is the only instance of Waddress-of-packed-variable in the tree for a user program that uses WARNS > 3. We suppress it completely for the kernel (D22876) and for programs using WARNS <= 3 (D22889).

I am open to any solution.

[imp]

In D22888#501061, @rlibby wrote:

In D22888#501055, @imp wrote:

It is actually properly aligned. Let's not do silly crap like this. Let's say it really is properly aligned.

It is indeed properly aligned, and gcc doesn't know it. How would you prefer to silence the warning? I felt it was just as much effort to do it in the Makefile.

I'd prefer to mark the buffer as properly aligned in the code.

[imp]

iff --git a/sbin/camcontrol/camcontrol.c b/sbin/camcontrol/camcontrol.c
index 4266c35ee03..79ab12e196b 100644
--- a/sbin/camcontrol/camcontrol.c
+++ b/sbin/camcontrol/camcontrol.c
@@ -2355,7 +2355,7 @@ ataidentify(struct cam_device *device, int retry_count, int timeout)
        if (arglist & CAM_ARG_VERBOSE) {
                printf("%s%d: Raw identify data:\n",
                    device->device_name, device->dev_unit_num);
-               dump_data((void*)ident_buf, sizeof(struct ata_params));
+               dump_data((uint16_t *)ident_buf, sizeof(struct ata_params));
        }

        if (ident_buf->support.command1 & ATA_SUPPORT_PROTECTED) {
diff --git a/sys/sys/ata.h b/sys/sys/ata.h
index 22edb5573d1..82d180b5491 100644
--- a/sys/sys/ata.h
+++ b/sys/sys/ata.h
@@ -311,7 +311,7 @@ struct ata_params {
 /*223*/ u_int16_t       transport_minor;
        u_int16_t       reserved224[31];
 /*255*/ u_int16_t       integrity;
-} __packed;
+} __packed __aligned(2);

 /* ATA Dataset Management */
 #define ATA_DSM_BLK_SIZE       512

I think is the right fix.

[rlibby]

Ah. Yes. I'll check if that builds. Thanks.

[imp]

In D22888#501137, @rlibby wrote:

Ah. Yes. I'll check if that builds. Thanks.

It builds for me with clang 9.0 :)

[rlibby]

imp's suggestion

[imp]

So it worked! Yippy! Of course I'd love you to commit this :)

[rlibby]

In D22888#501195, @imp wrote:

So it worked! Yippy! Of course I'd love you to commit this :)

Yep. Okay, will do.

I don't know the commit log etiquette here, committing your suggestion, will this be okay?

Submitted by:	imp
Reviewed by:	imp
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D22888