Re: [Qemu-devel] [PATCH v3 07/23] ui/vnc: Split out authentication_failure

[Philippe Mathieu-Daudé]

On 3/15/19 4:26 AM, Richard Henderson wrote:
> There were 3 copies of this code, one of which used the wrong
> data size for the failure indicator.

Which, as the comment said, has never been called.

Good cleanup.

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

> Cc: Gerd Hoffmann <address@hidden>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  ui/vnc.c | 37 +++++++++++++++----------------------
>  1 file changed, 15 insertions(+), 22 deletions(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 1871422e1d..785edf3af1 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -2535,6 +2535,18 @@ void start_client_init(VncState *vs)
>      vnc_read_when(vs, protocol_client_init, 1);
>  }
>  
> +static void authentication_failed(VncState *vs)
> +{
> +    vnc_write_u32(vs, 1); /* Reject auth */
> +    if (vs->minor >= 8) {
> +        static const char err[] = "Authentication failed";
> +        vnc_write_u32(vs, sizeof(err));
> +        vnc_write(vs, err, sizeof(err));
> +    }
> +    vnc_flush(vs);
> +    vnc_client_error(vs);
> +}
> +
>  static void make_challenge(VncState *vs)
>  {
>      int i;
> @@ -2609,14 +2621,7 @@ static int protocol_client_auth_vnc(VncState *vs, 
> uint8_t *data, size_t len)
>      return 0;
>  
>  reject:
> -    vnc_write_u32(vs, 1); /* Reject auth */
> -    if (vs->minor >= 8) {
> -        static const char err[] = "Authentication failed";
> -        vnc_write_u32(vs, sizeof(err));
> -        vnc_write(vs, err, sizeof(err));
> -    }
> -    vnc_flush(vs);
> -    vnc_client_error(vs);
> +    authentication_failed(vs);
>      qcrypto_cipher_free(cipher);
>      return 0;
>  }
> @@ -2638,13 +2643,7 @@ static int protocol_client_auth(VncState *vs, uint8_t 
> *data, size_t len)
>       * must pick the one we sent. Verify this */
>      if (data[0] != vs->auth) { /* Reject auth */
>         trace_vnc_auth_reject(vs, vs->auth, (int)data[0]);
> -       vnc_write_u32(vs, 1);
> -       if (vs->minor >= 8) {
> -           static const char err[] = "Authentication failed";
> -           vnc_write_u32(vs, sizeof(err));
> -           vnc_write(vs, err, sizeof(err));
> -       }
> -       vnc_client_error(vs);
> +       authentication_failed(vs);
>      } else { /* Accept requested auth */
>         trace_vnc_auth_start(vs, vs->auth);
>         switch (vs->auth) {
> @@ -2673,13 +2672,7 @@ static int protocol_client_auth(VncState *vs, uint8_t 
> *data, size_t len)
>  
>         default: /* Should not be possible, but just in case */
>             trace_vnc_auth_fail(vs, vs->auth, "Unhandled auth method", "");
> -           vnc_write_u8(vs, 1);
> -           if (vs->minor >= 8) {
> -               static const char err[] = "Authentication failed";
> -               vnc_write_u32(vs, sizeof(err));
> -               vnc_write(vs, err, sizeof(err));
> -           }
> -           vnc_client_error(vs);
> +           authentication_failed(vs);
>         }
>      }
>      return 0;
>