[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Mon, Sep 16, 2019 at 02:29:38PM -0500, Eric Blake wrote:
> 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;
>    }
> 

ACK

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]