[Spice-devel] [PATCH spice-common] canvas: Prevent some error compiling spice-gtk

[Uri Lublin]

On 01/23/2018 01:27 PM, Frediano Ziglio wrote:
> ping
> 
> Currently master is failing to compiler, see
> https://gitlab.com/spice/spice-gtk/-/jobs
> 
> This is supposed to be the patch in spice-gtk that take the spice-common changes:
> https://lists.freedesktop.org/archives/spice-devel/2018-January/041472.html
> 
> Frediano
> 
>>
>> Due to different warning setting some GCC reports:
>>
>> In file included from ../spice-common/common/sw_canvas.c:27:0,
>>                   from client_sw_canvas.c:20:
>> ../spice-common/common/canvas_base.c: In function ‘canvas_get_lz’:
>> ../spice-common/common/canvas_base.c:768:13: error: ‘palette’ may be used
>> uninitialized in this function [-Werror=maybe-uninitialized]
>>               free(palette);
>>               ^~~~~~~~~~~~~
>> ../spice-common/common/canvas_base.c:764:9: error: variable ‘free_palette’
>> might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
>>       int free_palette = FALSE;
>>           ^~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>>
>> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
>> ---
>>   common/canvas_base.c | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/common/canvas_base.c b/common/canvas_base.c
>> index 42f0eea..90d1d38 100644
>> --- a/common/canvas_base.c
>> +++ b/common/canvas_base.c
>> @@ -754,14 +754,14 @@ static pixman_image_t *canvas_get_lz(CanvasBase
>> *canvas, SpiceImage *image,
>>       uint8_t    *decomp_buf = NULL;
>>       pixman_format_code_t pixman_format;
>>       LzImageType type, as_type;
>> -    SpicePalette *palette;
>> +    SpicePalette *palette = NULL;
>>       int n_comp_pixels;
>>       int width;
>>       int height;
>>       int top_down;
>>       int stride_encoded;
>>       int stride;
>> -    int free_palette = FALSE;
>> +    volatile int free_palette = FALSE;

Hi Frediano,
Why does free_palette have to be volatile ?
Isn't it enough to initialize it ?
What's special with free_palette compared to other
variables ?

You may want to remove the free_palette = FALSE; line that
exists few lines below in the code (not shown here).

Do we need to add an if (free_palette) free() to
the if setjmp() block ?

Thanks,
     Uri.

>>   
>>       if (setjmp(lz_data->jmp_env)) {
>>           if (free_palette)  {
>> @@ -781,7 +781,9 @@ static pixman_image_t *canvas_get_lz(CanvasBase *canvas,
>> SpiceImage *image,
>>           spice_return_val_if_fail(image->u.lz_plt.data->num_chunks == 1,
>>           NULL); /* TODO: Handle chunks */
>>           comp_buf = image->u.lz_plt.data->chunk[0].data;
>>           comp_size = image->u.lz_plt.data->chunk[0].len;
>> -        palette = canvas_get_localized_palette(canvas,
>> image->u.lz_plt.palette, image->u.lz_plt.palette_id, image->u.lz_plt.flags,
>> &free_palette);
>> +        palette = canvas_get_localized_palette(canvas,
>> image->u.lz_plt.palette,
>> +                                               image->u.lz_plt.palette_id,
>> image->u.lz_plt.flags,
>> +                                               (int*) &free_palette);
>>       } else {
>>           spice_warn_if_reached();
>>           return NULL;