Re: [Libguestfs] [libnbd PATCH] states: Avoid magic number for h->tls

When we moved to an enum instead of raw int for nbd_set_tls(), we should have also updated our code to prefer the enum values. While at it, improve the grammar of error messages (confusing since 632196ec, and copy-and-pasted into more locations since then). Fixes: 4488cf2a Thanks: Rich Jones --- Rich noticed this while reviewing the patch for today's CVE fix. It's not a show-stopper if this doesn't get included in today's releases. generator/states-newstyle-opt-starttls.c | 8 ++++---- generator/states-newstyle.c | 4 ++-- generator/states-oldstyle.c | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/generator/states-newstyle-opt-starttls.c b/generator/states-newstyle-opt-starttls.c index 0a18db0..b050ce0 100644 --- a/generator/states-newstyle-opt-starttls.c +++ b/generator/states-newstyle-opt-starttls.c @@ -21,7 +21,7 @@ /* STATE MACHINE */ { NEWSTYLE.OPT_STARTTLS.START: /* If TLS was not requested we skip this option and go to the next one. */ - if (!h->tls) { + if (h->tls == LIBNBD_TLS_DISABLE) { SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); return 0; } @@ -88,13 +88,13 @@ return 0; } - /* Server refused to upgrade to TLS. If h->tls is not require (2) + /* Server refused to upgrade to TLS. If h->tls is not 'require' (2) * then we can continue unencrypted. */ - if (h->tls == 2) { + if (h->tls == LIBNBD_TLS_REQUIRE) { SET_NEXT_STATE (%.DEAD); set_error (ENOTSUP, "handshake: server refused TLS, " - "but handle TLS setting is require (2)"); + "but handle TLS setting is 'require' (2)"); return 0; } diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c index c8f817e..b4f2b80 100644 --- a/generator/states-newstyle.c +++ b/generator/states-newstyle.c @@ -129,10 +129,10 @@ handle_reply_error (struct nbd_handle *h) h->gflags = be16toh (h->gflags); if ((h->gflags & NBD_FLAG_FIXED_NEWSTYLE) == 0 && - h->tls == 2) { + h->tls == LIBNBD_TLS_REQUIRE) { SET_NEXT_STATE (%.DEAD); set_error (ENOTSUP, "handshake: server is not fixed newstyle, " - "but handle TLS setting is require (2)"); + "but handle TLS setting is 'require' (2)"); return 0; } diff --git a/generator/states-oldstyle.c b/generator/states-oldstyle.c index 1aff185..babefc0 100644 --- a/generator/states-oldstyle.c +++ b/generator/states-oldstyle.c @@ -46,13 +46,13 @@ gflags = be16toh (h->sbuf.old_handshake.gflags); eflags = be16toh (h->sbuf.old_handshake.eflags); - /* Server is unable to upgrade to TLS. If h->tls is not require (2) + /* Server is unable to upgrade to TLS. If h->tls is not 'require' (2) * then we can continue unencrypted. */ - if (h->tls == 2) { + if (h->tls == LIBNBD_TLS_REQUIRE) { SET_NEXT_STATE (%.DEAD); set_error (ENOTSUP, "handshake: server is oldstyle, " - "but handle TLS setting is require (2)"); + "but handle TLS setting is 'require' (2)"); return 0; }