9:17 p.m.
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(a)redhat.com>
Signed-off-by: Eric Blake <eblake(a)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