On Fri, Nov 30, 2018 at 04:03:34PM -0600, Eric Blake wrote:
There's no need to read into a temporary buffer (oversized
since commit 7d3123e1) followed by a byteswap into a uint64_t
to check for a magic number via memcmp(), when the code
immediately below demonstrates reading into the uint64_t then
byteswapping in place and checking for a magic number via
integer math. What's more, having a different error message
when the server's first reply byte is 0 is unusual - it's no
different from any other wrong magic number, and we already
detected short reads.
Signed-off-by: Eric Blake <address@hidden>
---
-
- buf[8] = '\0';
- if (strlen(buf) == 0) {
- error_setg(errp, "Server connection closed unexpectedly");
- goto fail;
- }
-
- magic = ldq_be_p(buf);
+ magic = be64_to_cpu(magic);
trace_nbd_receive_negotiate_magic(magic);
- if (memcmp(buf, "NBDMAGIC", 8) != 0) {
+ if (magic != NBD_INIT_MAGIC) {
error_setg(errp, "Invalid magic received");
goto fail;
}
The original code is really odd. What's the whole strlen about?