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

[Snir Sheriber]

Hi


On 08/11/2017 06:37 PM, Marc-André Lureau wrote:
> 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.

Why doesn't gcc complain also on the integers? ,"%s-%d:%d"
may still be more than 16 bytes if integers are large enough
(not here though, but it doesn't seem gcc check for possible integer
values...)
Anyway, "unknown" seems good enough to me..

>
> Thanks
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel