[Spice-devel] [PATCH spice-gtk 1/6] channel: return "unknown" in spice_channel_type_to_string()

[Marc-André Lureau]

Hi

----- Original Message -----
> > 
> > From: Marc-André Lureau <marcandre.lureau at redhat.com>
> > 
> > This fixes the following gcc warning:
> > 
> > spice-channel.c: In function ‘spice_channel_constructed’:
> > spice-channel.c:144:41: error: ‘%s’ directive output may be truncated
> > writing
> > likely 20 or more bytes into a region of size 16
> > [-Werror=format-truncation=]
> >      snprintf(c->name, sizeof(c->name), "%s-%d:%d",
> >                                          ^~
> > spice-channel.c:144:40: note: assuming directive output of 20 bytes
> >      snprintf(c->name, sizeof(c->name), "%s-%d:%d",
> > 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> > ---
> >  src/spice-channel.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/spice-channel.c b/src/spice-channel.c
> > index 4c3db9d..7cd6251 100644
> > --- a/src/spice-channel.c
> > +++ b/src/spice-channel.c
> > @@ -2130,13 +2130,13 @@ static const char *to_string[] = {
> >   **/
> >  const gchar* spice_channel_type_to_string(gint type)
> >  {
> > -    const char *str = NULL;
> > +    const char *str = "unknown";
> >  
> >      if (type >= 0 && type < G_N_ELEMENTS(to_string)) {
> >          str = to_string[type];
> >      }
> >  
> > -    return str ? str : "unknown channel type";
> > +    return str;
> >  }
> >  
> >  /**
> 
> Note that some array entries contains NULL so changing the code
> that way you are returning NULL in some cases while before
> was never NULL. Simply modifying the string solve the gcc issue

oops, my bad.

At first I thought the function should return NULL instead, but it is API since 0.20. It is likely that is only used to print a user friendly message. I think we made the wrong choice. Although it should be fine to change to return NULL, and updating the doc. Then in various places, either have gprintf %s/null behaviour, or we have to_string() ?: "unknown"..

> without this regression. Snir was working in the same issue but
> you arrived first with a patch.
> Potentially this change the reply to a public API. Maybe to avoid
> potentially having an "unknown" channel we could use "UNKNOWN"
> or "Unknown" ?

I'll just modify the patch to return "unknown" for now.

Thanks