[PATCH weston v2 8/8] terminal: Silence maybe-uninitialized warning

[Quentin Glidic]

On 11/07/2016 23:28, Emil Velikov wrote:
> On 10 July 2016 at 10:00, Quentin Glidic
> <sardemff7+wayland at sardemff7.net> wrote:
>> From: Quentin Glidic <sardemff7+git at sardemff7.net>
>>
>> clients/terminal.c: In function 'redraw_handler':
>> clients/terminal.c:213:28: warning: 'machine.unicode' may be used
>> uninitialized in this function [-Wmaybe-uninitialized]
>>   struct utf8_state_machine machine;
>>                             ^~~~~~~
>>
>> clients/terminal.c: In function 'handle_char':
>> clients/terminal.c:213:28: warning: 'machine.unicode' may be used
>> uninitialized in this function [-Wmaybe-uninitialized]
>>
>> Warning produced by GCC 5.3 and 6.1, with -O3.
>>
>> Signed-off-by: Quentin Glidic <sardemff7+git at sardemff7.net>
>> ---
>>
>> You have to look at the file directly to completely understand this one.
>> Not sure that is the value to return in this case, but at least it
>> silences the warning.
>>
> ::unicode will never be read (get_unicode) when it's invalid, since
> ::state will equal to utf8state_reject (as set in utf8_next_char).
> Seems like the compiler/optimiser cannot see that far, thus throws a
> warning message.
>
> With anything vaguely like the above in the commit message, the patch is
> Reviewed-by: Emil Velikov <emil.l.velikov at gmail.com>

I found it weird that the compiler wouldn’t see that, so I re-checked 
the code.

I think with -O3, this specific "for" is compile-time unlooped, and 
utf8_next_char inlined.
And there is *one* path that can keep machine.state to utf8state_start, 
thus triggering the warning.

Without -O3, the function is globally tagged as “changing unicode”, so 
no warning is produced.

I will let the pusher to choose what to include in the commit message, 
as I find the wording “silence” (and not “fix”) clear enough, though in 
this specific case, it could need an actual fix.

Side note: I picked 0 as the default value, but maybe in this case 
0xfffd would be better?


Cheers,

-- 

Quentin “Sardem FF7” Glidic