[Libguestfs] [libnbd PATCH v4 1/4] states: Document our reliance on type overlaps

[Eric Blake]

When I added structured replies to the NBD spec, I intentionally chose
a wire layout where the magic number and cookie overlap, even while
the middle member changes from uint32_t error to the pair uint16_t
flags and type.  Based only on a strict reading of C rules on
effective types and compatible type prefixes, it's probably
questionable on whether my reliance on type aliasing to reuse cookie
from the same offset of a union, or even the fact that a structured
reply is built by first reading bytes into sbuf.simple_reply then
following up with only bytes into the tail of sbuf.sr.structured_reply
is strictly portable.  But since it works in practice, it's worth at
least adding some compile- and run-time assertions that our (ab)use of
aliasing is accessing the bytes we want under the types we expect.
Upcoming patches will restructure part of the sbuf layout to hopefully
be a little easier to tie back to strict C standards.

Suggested-by: Laszlo Ersek <lersek at redhat.com>
Signed-off-by: Eric Blake <eblake at redhat.com>
---
 generator/states-reply.c            | 17 +++++++++++++----
 generator/states-reply-structured.c | 13 +++++++++----
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/generator/states-reply.c b/generator/states-reply.c
index 511e5cb1..2c77658b 100644
--- a/generator/states-reply.c
+++ b/generator/states-reply.c
@@ -17,6 +17,7 @@
  */

 #include <assert.h>
+#include <stddef.h>

 /* State machine for receiving reply messages from the server.
  *
@@ -63,9 +64,15 @@  REPLY.START:
   ssize_t r;

   /* We read all replies initially as if they are simple replies, but
-   * check the magic in CHECK_SIMPLE_OR_STRUCTURED_REPLY below.
-   * This works because the structured_reply header is larger.
+   * check the magic in CHECK_SIMPLE_OR_STRUCTURED_REPLY below.  This
+   * works because the structured_reply header is larger, and because
+   * the last member of a simple reply, cookie, is coincident between
+   * the two structs (an intentional design decision in the NBD spec
+   * when structured replies were added).
    */
+  STATIC_ASSERT (offsetof (struct nbd_handle, sbuf.simple_reply.cookie) ==
+                 offsetof (struct nbd_handle, sbuf.sr.structured_reply.cookie),
+                 cookie_aliasing);
   assert (h->reply_cmd == NULL);
   assert (h->rlen == 0);

@@ -135,7 +142,8 @@  REPLY.CHECK_SIMPLE_OR_STRUCTURED_REPLY:
   }

   /* NB: This works for both simple and structured replies because the
-   * handle (our cookie) is stored at the same offset.
+   * handle (our cookie) is stored at the same offset.  See the
+   * STATIC_ASSERT above in state REPLY.START that confirmed this.
    */
   h->chunks_received++;
   cookie = be64toh (h->sbuf.simple_reply.cookie);
@@ -155,7 +163,8 @@  REPLY.FINISH_COMMAND:
   bool retire;

   /* NB: This works for both simple and structured replies because the
-   * handle (our cookie) is stored at the same offset.
+   * handle (our cookie) is stored at the same offset.  See the
+   * STATIC_ASSERT above in state REPLY.START that confirmed this.
    */
   cookie = be64toh (h->sbuf.simple_reply.cookie);
   /* Find the command amongst the commands in flight. */
diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c
index 5aca7262..205a236d 100644
--- a/generator/states-reply-structured.c
+++ b/generator/states-reply-structured.c
@@ -19,6 +19,7 @@
 /* State machine for parsing structured replies from the server. */

 #include <stdbool.h>
+#include <stddef.h>
 #include <stdint.h>
 #include <inttypes.h>

@@ -45,11 +46,15 @@ structured_reply_in_bounds (uint64_t offset, uint32_t length,

 STATE_MACHINE {
  REPLY.STRUCTURED_REPLY.START:
-  /* We've only read the simple_reply.  The structured_reply is longer,
-   * so read the remaining part.
+  /* We've only read the bytes that fill simple_reply.  The
+   * structured_reply is longer, so read the remaining part.  We
+   * depend on the memory aliasing in union sbuf to overlay the two
+   * reply types.
    */
-  h->rbuf = &h->sbuf;
-  h->rbuf = (char *)h->rbuf + sizeof h->sbuf.simple_reply;
+  STATIC_ASSERT (sizeof h->sbuf.simple_reply ==
+                 offsetof (struct nbd_structured_reply, length),
+                 simple_structured_overlap);
+  assert (h->rbuf == (char *)&h->sbuf + sizeof h->sbuf.simple_reply);
   h->rlen = sizeof h->sbuf.sr.structured_reply;
   h->rlen -= sizeof h->sbuf.simple_reply;
   SET_NEXT_STATE (%RECV_REMAINING);
-- 
2.40.1