[2/2] fix warning: variable X might be clobbered by 'longjmp'

Submitted by Uli Schlachter on Dec. 23, 2017, 1:16 p.m.

Details

Message ID 20171223131608.19011-2-psychon@znc.in
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Cairo

Not browsing as part of any series.

Commit Message

Uli Schlachter Dec. 23, 2017, 1:16 p.m.
According to "man setjmp", one possible way to avoid variable clobbering
is to declare them as volatile. Thus, this commit turns the variables
that are changed between setjmp() and longjmp() and whose values are
still needed after setjmp() returned the second time into volatile
variables.

The warning in cairo-bentley-ottmann-rectangular.c is worked around by
not initializing the variable before setjmp(). To be honest, I don't
understand why the compiler warns here at all since the value of update
is clearly not used after setjmp()'s second return.

This commit re-fixes the warnings that were reintroduced in commit
82f40285 which reverted b092b63.

Signed-off-by: Uli Schlachter <psychon@znc.in>
---
 src/cairo-bentley-ottmann-rectangular.c | 4 +++-
 src/cairo-png.c                         | 6 +++---
 2 files changed, 6 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/cairo-bentley-ottmann-rectangular.c b/src/cairo-bentley-ottmann-rectangular.c
index 29f902c1a..65f95d797 100644
--- a/src/cairo-bentley-ottmann-rectangular.c
+++ b/src/cairo-bentley-ottmann-rectangular.c
@@ -603,7 +603,7 @@  _cairo_bentley_ottmann_tessellate_rectangular (rectangle_t	**rectangles,
     sweep_line_t sweep_line;
     rectangle_t *rectangle;
     cairo_status_t status;
-    cairo_bool_t update = FALSE;
+    cairo_bool_t update;
 
     sweep_line_init (&sweep_line,
 		     rectangles, num_rectangles,
@@ -612,6 +612,8 @@  _cairo_bentley_ottmann_tessellate_rectangular (rectangle_t	**rectangles,
     if ((status = setjmp (sweep_line.unwind)))
 	return status;
 
+    update = FALSE;
+
     rectangle = rectangle_pop_start (&sweep_line);
     do {
 	if (rectangle->top != sweep_line.current_y) {
diff --git a/src/cairo-png.c b/src/cairo-png.c
index 596b506ab..ab0b9d0c5 100644
--- a/src/cairo-png.c
+++ b/src/cairo-png.c
@@ -544,11 +544,11 @@  stream_read_func (png_structp png, png_bytep data, png_size_t size)
 static cairo_surface_t *
 read_png (struct png_read_closure_t *png_closure)
 {
-    cairo_surface_t *surface;
+    cairo_surface_t * volatile surface;
     png_struct *png = NULL;
     png_info *info;
-    png_byte *data = NULL;
-    png_byte **row_pointers = NULL;
+    png_byte * volatile data = NULL;
+    png_byte ** volatile row_pointers = NULL;
     png_uint_32 png_width, png_height;
     int depth, color_type, interlace, stride;
     unsigned int i;

Comments

On Sat, Dec 23, 2017 at 02:16:08PM +0100, Uli Schlachter wrote:
> According to "man setjmp", one possible way to avoid variable clobbering
> is to declare them as volatile. Thus, this commit turns the variables
> that are changed between setjmp() and longjmp() and whose values are
> still needed after setjmp() returned the second time into volatile
> variables.
> 
> The warning in cairo-bentley-ottmann-rectangular.c is worked around by
> not initializing the variable before setjmp(). To be honest, I don't
> understand why the compiler warns here at all since the value of update
> is clearly not used after setjmp()'s second return.
> 
> This commit re-fixes the warnings that were reintroduced in commit
> 82f40285 which reverted b092b63.
> 
> Signed-off-by: Uli Schlachter <psychon@znc.in>

I don't know all the intricacies of volatile and setjmp(), but trust if
it quells the compiler warning it must be good, right?

Acked-by: Bryce Harrington <bryce@osg.samsung.com>

> ---
>  src/cairo-bentley-ottmann-rectangular.c | 4 +++-
>  src/cairo-png.c                         | 6 +++---
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/src/cairo-bentley-ottmann-rectangular.c b/src/cairo-bentley-ottmann-rectangular.c
> index 29f902c1a..65f95d797 100644
> --- a/src/cairo-bentley-ottmann-rectangular.c
> +++ b/src/cairo-bentley-ottmann-rectangular.c
> @@ -603,7 +603,7 @@ _cairo_bentley_ottmann_tessellate_rectangular (rectangle_t	**rectangles,
>      sweep_line_t sweep_line;
>      rectangle_t *rectangle;
>      cairo_status_t status;
> -    cairo_bool_t update = FALSE;
> +    cairo_bool_t update;
>  
>      sweep_line_init (&sweep_line,
>  		     rectangles, num_rectangles,
> @@ -612,6 +612,8 @@ _cairo_bentley_ottmann_tessellate_rectangular (rectangle_t	**rectangles,
>      if ((status = setjmp (sweep_line.unwind)))
>  	return status;
>  
> +    update = FALSE;
> +
>      rectangle = rectangle_pop_start (&sweep_line);
>      do {
>  	if (rectangle->top != sweep_line.current_y) {
> diff --git a/src/cairo-png.c b/src/cairo-png.c
> index 596b506ab..ab0b9d0c5 100644
> --- a/src/cairo-png.c
> +++ b/src/cairo-png.c
> @@ -544,11 +544,11 @@ stream_read_func (png_structp png, png_bytep data, png_size_t size)
>  static cairo_surface_t *
>  read_png (struct png_read_closure_t *png_closure)
>  {
> -    cairo_surface_t *surface;
> +    cairo_surface_t * volatile surface;
>      png_struct *png = NULL;
>      png_info *info;
> -    png_byte *data = NULL;
> -    png_byte **row_pointers = NULL;
> +    png_byte * volatile data = NULL;
> +    png_byte ** volatile row_pointers = NULL;
>      png_uint_32 png_width, png_height;
>      int depth, color_type, interlace, stride;
>      unsigned int i;
> -- 
> 2.15.1
> 
> -- 
> cairo mailing list
> cairo@cairographics.org
> https://lists.cairographics.org/mailman/listinfo/cairo
On 05.01.2018 02:54, Bryce Harrington wrote:
> On Sat, Dec 23, 2017 at 02:16:08PM +0100, Uli Schlachter wrote:
>> According to "man setjmp", one possible way to avoid variable clobbering
>> is to declare them as volatile. Thus, this commit turns the variables
>> that are changed between setjmp() and longjmp() and whose values are
>> still needed after setjmp() returned the second time into volatile
>> variables.
>>
>> The warning in cairo-bentley-ottmann-rectangular.c is worked around by
>> not initializing the variable before setjmp(). To be honest, I don't
>> understand why the compiler warns here at all since the value of update
>> is clearly not used after setjmp()'s second return.
>>
>> This commit re-fixes the warnings that were reintroduced in commit
>> 82f40285 which reverted b092b63.
>>
>> Signed-off-by: Uli Schlachter <psychon@znc.in>
> 
> I don't know all the intricacies of volatile and setjmp(), but trust if
> it quells the compiler warning it must be good, right?

By that reasoning, the original patch that I reverted since it caused
endless loops was already good. ;-)

Thanks for taking a look. I just pushed both patches.

> Acked-by: Bryce Harrington <bryce@osg.samsung.com>
> 
>> ---
>>  src/cairo-bentley-ottmann-rectangular.c | 4 +++-
>>  src/cairo-png.c                         | 6 +++---
>>  2 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/cairo-bentley-ottmann-rectangular.c b/src/cairo-bentley-ottmann-rectangular.c
>> index 29f902c1a..65f95d797 100644
>> --- a/src/cairo-bentley-ottmann-rectangular.c
>> +++ b/src/cairo-bentley-ottmann-rectangular.c
>> @@ -603,7 +603,7 @@ _cairo_bentley_ottmann_tessellate_rectangular (rectangle_t	**rectangles,
>>      sweep_line_t sweep_line;
>>      rectangle_t *rectangle;
>>      cairo_status_t status;
>> -    cairo_bool_t update = FALSE;
>> +    cairo_bool_t update;
>>  
>>      sweep_line_init (&sweep_line,
>>  		     rectangles, num_rectangles,
>> @@ -612,6 +612,8 @@ _cairo_bentley_ottmann_tessellate_rectangular (rectangle_t	**rectangles,
>>      if ((status = setjmp (sweep_line.unwind)))
>>  	return status;
>>  
>> +    update = FALSE;
>> +
>>      rectangle = rectangle_pop_start (&sweep_line);
>>      do {
>>  	if (rectangle->top != sweep_line.current_y) {
>> diff --git a/src/cairo-png.c b/src/cairo-png.c
>> index 596b506ab..ab0b9d0c5 100644
>> --- a/src/cairo-png.c
>> +++ b/src/cairo-png.c
>> @@ -544,11 +544,11 @@ stream_read_func (png_structp png, png_bytep data, png_size_t size)
>>  static cairo_surface_t *
>>  read_png (struct png_read_closure_t *png_closure)
>>  {
>> -    cairo_surface_t *surface;
>> +    cairo_surface_t * volatile surface;
>>      png_struct *png = NULL;
>>      png_info *info;
>> -    png_byte *data = NULL;
>> -    png_byte **row_pointers = NULL;
>> +    png_byte * volatile data = NULL;
>> +    png_byte ** volatile row_pointers = NULL;
>>      png_uint_32 png_width, png_height;
>>      int depth, color_type, interlace, stride;
>>      unsigned int i;
>> -- 
>> 2.15.1
>>
>> -- 
>> cairo mailing list
>> cairo@cairographics.org
>> https://lists.cairographics.org/mailman/listinfo/cairo