[Spice-devel] [PATCH v5 9/9] Remove message_buf from EncoderData

[Jonathon Jongsma]

On Wed, 2016-06-15 at 10:37 +0100, Frediano Ziglio wrote:
> This buffer was just written and then used, no reason to store into
> a more persistent structure.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/dcc-encoders.c | 34 ++++++++++++++++++----------------
>  server/dcc-encoders.h |  1 -
>  2 files changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index fda3b83..cec0c51 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -83,11 +83,12 @@ quic_usr_error(QuicUsrContext *usr, const char *fmt, ...)
>  {
>      EncoderData *usr_data = &(((QuicData *)usr)->data);
>      va_list ap;
> +    char message_buf[512];

Would prefer something like:
#define ENCODER_MESSAGE_SIZE 512


>  
>      va_start(ap, fmt);
> -    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt, ap);
> +    vsnprintf(message_buf, sizeof(message_buf), fmt, ap);
>      va_end(ap);
> -    spice_critical("%s", usr_data->message_buf);
> +    spice_critical("%s", message_buf);
>  
>      longjmp(usr_data->jmp_env, 1);
>  }
> @@ -97,11 +98,12 @@ lz_usr_error(LzUsrContext *usr, const char *fmt, ...)
>  {
>      EncoderData *usr_data = &(((LzData *)usr)->data);
>      va_list ap;
> +    char message_buf[512];
>  
>      va_start(ap, fmt);
> -    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt, ap);
> +    vsnprintf(message_buf, sizeof(message_buf), fmt, ap);
>      va_end(ap);
> -    spice_critical("%s", usr_data->message_buf);
> +    spice_critical("%s", message_buf);
>  
>      longjmp(usr_data->jmp_env, 1);
>  }
> @@ -109,14 +111,14 @@ lz_usr_error(LzUsrContext *usr, const char *fmt, ...)
>  static SPICE_GNUC_PRINTF(2, 3) void
>  glz_usr_error(GlzEncoderUsrContext *usr, const char *fmt, ...)
>  {
> -    EncoderData *usr_data = &(((GlzData *)usr)->data);
>      va_list ap;
> +    char message_buf[512];
>  
>      va_start(ap, fmt);
> -    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt, ap);
> +    vsnprintf(message_buf, sizeof(message_buf), fmt, ap);
>      va_end(ap);
>  
> -    spice_critical("%s", usr_data->message_buf); // if global lz fails in the
> middle
> +    spice_critical("%s", message_buf); // if global lz fails in the middle
>                                          // the consequences are not
> predictable since the window
>                                          // can turn to be unsynchronized
> between the server and
>                                          // and the client
> @@ -125,37 +127,37 @@ glz_usr_error(GlzEncoderUsrContext *usr, const char
> *fmt, ...)
>  static SPICE_GNUC_PRINTF(2, 3) void
>  quic_usr_warn(QuicUsrContext *usr, const char *fmt, ...)
>  {
> -    EncoderData *usr_data = &(((QuicData *)usr)->data);
>      va_list ap;
> +    char message_buf[512];
>  
>      va_start(ap, fmt);
> -    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt, ap);
> +    vsnprintf(message_buf, sizeof(message_buf), fmt, ap);
>      va_end(ap);
> -    spice_warning("%s", usr_data->message_buf);
> +    spice_warning("%s", message_buf);
>  }
>  
>  static SPICE_GNUC_PRINTF(2, 3) void
>  lz_usr_warn(LzUsrContext *usr, const char *fmt, ...)
>  {
> -    EncoderData *usr_data = &(((LzData *)usr)->data);
>      va_list ap;
> +    char message_buf[512];
>  
>      va_start(ap, fmt);
> -    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt, ap);
> +    vsnprintf(message_buf, sizeof(message_buf), fmt, ap);
>      va_end(ap);
> -    spice_warning("%s", usr_data->message_buf);
> +    spice_warning("%s", message_buf);
>  }
>  
>  static SPICE_GNUC_PRINTF(2, 3) void
>  glz_usr_warn(GlzEncoderUsrContext *usr, const char *fmt, ...)
>  {
> -    EncoderData *usr_data = &(((GlzData *)usr)->data);
>      va_list ap;
> +    char message_buf[512];
>  
>      va_start(ap, fmt);
> -    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt, ap);
> +    vsnprintf(message_buf, sizeof(message_buf), fmt, ap);
>      va_end(ap);
> -    spice_warning("%s", usr_data->message_buf);
> +    spice_warning("%s", message_buf);
>  }
>  
>  static void *quic_usr_malloc(QuicUsrContext *usr, int size)
> diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
> index efe861f..0f69cd0 100644
> --- a/server/dcc-encoders.h
> +++ b/server/dcc-encoders.h
> @@ -98,7 +98,6 @@ typedef struct  {
>              int size_left;
>          } compressed_data; // for encoding data that was already compressed
> by another method
>      } u;
> -    char message_buf[512];
>  } EncoderData;
>  
>  typedef struct {


Acked-by: Jonathon Jongsma <jjongsma at redhat.com>