[Spice-devel] [spice-gtk v1 1/3] Improve debug log for preferred compression message

[Victor Toso]

Hi,

On Wed, Dec 20, 2017 at 09:41:30AM -0500, Frediano Ziglio wrote:
> > 
> > On 12/20/2017 03:18 PM, Victor Toso wrote:
> > > From: Victor Toso <me at victortoso.com>
> > 
> > Hi Victor,
> > 
> > > 
> > > To use a string instead of number (enum)
> > > 
> > > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > > ---
> > >   src/channel-display.c | 26 +++++++++++++++++++++++++-
> > >   1 file changed, 25 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/src/channel-display.c b/src/channel-display.c
> > > index 75d2e32..dece3b9 100644
> > > --- a/src/channel-display.c
> > > +++ b/src/channel-display.c
> > > @@ -536,6 +536,29 @@ void
> > > spice_display_change_preferred_compression(SpiceChannel *channel, gint
> > > comp
> > >       spice_display_channel_change_preferred_compression(channel,
> > >       compression);
> > >   }
> > >   
> > > +static const gchar *image_compression_types_str[] = {
> > > +    [ SPICE_IMAGE_COMPRESSION_INVALID ] = "invalid",
> > > +    [ SPICE_IMAGE_COMPRESSION_OFF ] = "off",
> > > +    [ SPICE_IMAGE_COMPRESSION_AUTO_GLZ ] = "auto-glz",
> > > +    [ SPICE_IMAGE_COMPRESSION_AUTO_LZ ] = "auto-lz",
> > > +    [ SPICE_IMAGE_COMPRESSION_QUIC ] = "quic",
> > > +    [ SPICE_IMAGE_COMPRESSION_GLZ ] = "glz",
> > > +    [ SPICE_IMAGE_COMPRESSION_LZ ] = "lz",
> > > +    [ SPICE_IMAGE_COMPRESSION_LZ4 ] = "lz4",
> > > +};
> > > +G_STATIC_ASSERT(G_N_ELEMENTS(image_compression_types_str) <=
> > > SPICE_IMAGE_COMPRESSION_ENUM_END);
> > > +
> > > +static const gchar *preferred_compression_type_to_string(gint type)
> > > +{
> > > +    const char *str = NULL;
> > 
> > It would be simpler to initialize str = "unknown";
> > > +
> > > +    if (type >= 0 && type < G_N_ELEMENTS(image_compression_types_str)) {
> > > +        str = image_compression_types_str[type];
> > > +    }
> > > +
> > > +    return str ? str : "unknown";
> > 
> > And simply return str; here
> > 
> > Uri.
> > 
> 
> Does not work if there are holes in the array.
> 
> Maybe we should define an helper for this and have
> 
> const gchar *enum_value_to_string(gint value, const gchar *const *values, size_t num_values)
> {
>     const char *str = NULL;
> 
>     if (value >= 0 && value < num_values) {
>         str = values[value];
>     }
> 
>     return str ? str : "unknown";
> }
> 
> static const gchar *preferred_compression_type_to_string(gint type)
> {
>    return enum_value_to_string(type, image_compression_types_str, G_N_ELEMENTS(image_compression_types_str));
> }
> 
> ??

To be honest, I wrote this patch 3 times because of this. There
are other places in the source code that we do enum-to-string
conversion and I was thinking in making this better now too.

But then I thought that I was complicating things :)

I was thinking in a similar approach as you wrote above but if we
do that, we should make it in spice-common to benefit spice
server too, agree?

Cheers,
        toso

> 
> Frediano
> 
> > > +}
> > > +
> > >   /**
> > >    * spice_display_channel_change_preferred_compression:
> > >    * @channel: a #SpiceDisplayChannel
> > > @@ -560,7 +583,8 @@ void
> > > spice_display_channel_change_preferred_compression(SpiceChannel *channel,
> > > g
> > >           return;
> > >       }
> > >   
> > > -    CHANNEL_DEBUG(channel, "changing preferred compression to %d",
> > > compression);
> > > +    CHANNEL_DEBUG(channel, "changing preferred compression to %s",
> > > +                  preferred_compression_type_to_string(compression));
> > >   
> > >       pref_comp_msg.image_compression = compression;
> > >       out = spice_msg_out_new(channel,
> > >       SPICE_MSGC_DISPLAY_PREFERRED_COMPRESSION);
> > > 
> > 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20171220/3142e534/attachment.sig>