Re: RFC: 'unused return value' errors on RHEL 6

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> spice-bitmap-utils.c has a lot of calls to fwrite() that don't check the
> return value. On RHEL 6 these cause compilation errors due to -Werror:
> 
> spice-bitmap-utils.c:147: error: ignoring return value of 'fwrite',
> declared with attribute warn_unused_result
> 
> What should be done about that?
>  * One option would be to remove -Werror but that would be really
>    heavy handed.
> 
>  * I have not found a way to disable just that one warning. But if
>    others have more luck that could be an option.
> 
>  * The best solution would be to not ignore the return values but that
>    means adding a lot of error-handling code. Does anyone want to take
>    this on?
> 
>  * Otherwise the usual way to trick the compiler is to rewrite the code
>    as
>        (void)(func(...)+1)
>    It's really ugly so it is generally further hidden in a macro.
>    This would yield a patch somewhat like this:
> 
> 
> diff --git a/server/spice-bitmap-utils.c b/server/spice-bitmap-utils.c
> index 8d6e7c6..3cc8b35 100644
> --- a/server/spice-bitmap-utils.c
> +++ b/server/spice-bitmap-utils.c
> @@ -30,6 +30,8 @@
>  #define GRADUAL_HIGH_RGB24_TH -0.03
>  #define GRADUAL_HIGH_RGB16_TH 0
>  
> +#define IGNORE(x) ((void)(x+1))
> +
>  // setting a more permissive threshold for stream identification in order
>  // not to miss streams that were artificially scaled on the guest (e.g.,
>  full screen view
>  // in window media player 12). see red_stream_add_frame
> @@ -141,7 +143,7 @@ static void dump_palette(FILE *f, SpicePalette* plt)
>  {
>      int i;
>      for (i = 0; i < plt->num_ents; i++) {
> -        fwrite(plt->ents + i, sizeof(uint32_t), 1, f);
> +        IGNORE(fwrite(plt->ents + i, sizeof(uint32_t), 1, f));
>      }
>  }
>  
> @@ -150,11 +152,11 @@ static void dump_line(FILE *f, uint8_t* line, uint16_t
> n_pixel_bits, int width,
>      int i;
>      int copy_bytes_size = SPICE_ALIGN(n_pixel_bits * width, 8) / 8;
>  
> -    fwrite(line, 1, copy_bytes_size, f);
> +    IGNORE(fwrite(line, 1, copy_bytes_size, f));
>      if (row_size > copy_bytes_size) {
>          // each line should be 4 bytes aligned
>          for (i = copy_bytes_size; i < row_size; i++) {
> -            fprintf(f, "%c", 0);
> +            IGNORE(fprintf(f, "%c", 0));
>          }
>      }
>  }
> @@ -235,36 +237,36 @@ void dump_bitmap(SpiceBitmap *bitmap)
>      }
>  
>      /* writing the bmp v3 header */
> -    fprintf(f, "BM");
> -    fwrite(&file_size, sizeof(file_size), 1, f);
> +    IGNORE(fprintf(f, "BM"));
> +    IGNORE(fwrite(&file_size, sizeof(file_size), 1, f));
>      tmp_u16 = alpha ? 1 : 0;
> -    fwrite(&tmp_u16, sizeof(tmp_u16), 1, f); // reserved for application
> +    IGNORE(fwrite(&tmp_u16, sizeof(tmp_u16), 1, f)); // reserved for
> application
>      tmp_u16 = 0;
> -    fwrite(&tmp_u16, sizeof(tmp_u16), 1, f);
> -    fwrite(&bitmap_data_offset, sizeof(bitmap_data_offset), 1, f);
> +    IGNORE(fwrite(&tmp_u16, sizeof(tmp_u16), 1, f));
> +    IGNORE(fwrite(&bitmap_data_offset, sizeof(bitmap_data_offset), 1, f));
>      tmp_u32 = header_size - 14;
> -    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f); // sub header size
> +    IGNORE(fwrite(&tmp_u32, sizeof(tmp_u32), 1, f)); // sub header size
>      tmp_32 = bitmap->x;
> -    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
> +    IGNORE(fwrite(&tmp_32, sizeof(tmp_32), 1, f));
>      tmp_32 = bitmap->y;
> -    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
> +    IGNORE(fwrite(&tmp_32, sizeof(tmp_32), 1, f));
>  
>      tmp_u16 = 1;
> -    fwrite(&tmp_u16, sizeof(tmp_u16), 1, f); // color plane
> -    fwrite(&n_pixel_bits, sizeof(n_pixel_bits), 1, f); // pixel depth
> +    IGNORE(fwrite(&tmp_u16, sizeof(tmp_u16), 1, f)); // color plane
> +    IGNORE(fwrite(&n_pixel_bits, sizeof(n_pixel_bits), 1, f)); // pixel
> depth
>  
>      tmp_u32 = 0;
> -    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f); // compression method
> +    IGNORE(fwrite(&tmp_u32, sizeof(tmp_u32), 1, f)); // compression method
>  
>      tmp_u32 = 0; //file_size - bitmap_data_offset;
> -    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f); // image size
> +    IGNORE(fwrite(&tmp_u32, sizeof(tmp_u32), 1, f)); // image size
>      tmp_32 = 0;
> -    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
> -    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
> +    IGNORE(fwrite(&tmp_32, sizeof(tmp_32), 1, f));
> +    IGNORE(fwrite(&tmp_32, sizeof(tmp_32), 1, f));
>      tmp_u32 = (!plt) ? 0 : plt->num_ents; // plt entries
> -    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f);
> +    IGNORE(fwrite(&tmp_u32, sizeof(tmp_u32), 1, f));
>      tmp_u32 = 0;
> -    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f);
> +    IGNORE(fwrite(&tmp_u32, sizeof(tmp_u32), 1, f));
>  
>      if (plt) {
>          dump_palette(f, plt);

Other proposal, handle the error


diff --git a/server/spice-bitmap-utils.c b/server/spice-bitmap-utils.c
index 8d6e7c6..23644d9 100644
--- a/server/spice-bitmap-utils.c
+++ b/server/spice-bitmap-utils.c
@@ -137,27 +137,37 @@ int spice_bitmap_from_surface_type(uint32_t surface_format)
 
 #define RAM_PATH "/tmp/tmpfs"
 
-static void dump_palette(FILE *f, SpicePalette* plt)
+#define write(buf, size) do { \
+    if (fwrite(buf, 1, (size), f) != (size)) \
+        goto write_err; \
+} while(0)
+
+static bool dump_palette(FILE *f, SpicePalette* plt)
 {
     int i;
     for (i = 0; i < plt->num_ents; i++) {
-        fwrite(plt->ents + i, sizeof(uint32_t), 1, f);
+        write(plt->ents + i, sizeof(uint32_t));
     }
+    return true;
+
+write_err:
+    return false;
 }
 
-static void dump_line(FILE *f, uint8_t* line, uint16_t n_pixel_bits, int width, int row_size)
+static bool dump_line(FILE *f, uint8_t* line, uint16_t n_pixel_bits, int width, int row_size)
 {
-    int i;
+    static char zeroes[4] = { 0 };
     int copy_bytes_size = SPICE_ALIGN(n_pixel_bits * width, 8) / 8;
 
-    fwrite(line, 1, copy_bytes_size, f);
-    if (row_size > copy_bytes_size) {
-        // each line should be 4 bytes aligned
-        for (i = copy_bytes_size; i < row_size; i++) {
-            fprintf(f, "%c", 0);
-        }
-    }
+    write(line, copy_bytes_size);
+    // each line should be 4 bytes aligned
+    write(zeroes, row_size - copy_bytes_size);
+    return true;
+
+write_err:
+    return false;
 }
+
 void dump_bitmap(SpiceBitmap *bitmap)
 {
     static uint32_t file_id = 0;
@@ -235,47 +245,51 @@ void dump_bitmap(SpiceBitmap *bitmap)
     }
 
     /* writing the bmp v3 header */
-    fprintf(f, "BM");
-    fwrite(&file_size, sizeof(file_size), 1, f);
+    write("BM", 2);
+    write(&file_size, sizeof(file_size));
     tmp_u16 = alpha ? 1 : 0;
-    fwrite(&tmp_u16, sizeof(tmp_u16), 1, f); // reserved for application
+    write(&tmp_u16, sizeof(tmp_u16)); // reserved for application
     tmp_u16 = 0;
-    fwrite(&tmp_u16, sizeof(tmp_u16), 1, f);
-    fwrite(&bitmap_data_offset, sizeof(bitmap_data_offset), 1, f);
+    write(&tmp_u16, sizeof(tmp_u16));
+    write(&bitmap_data_offset, sizeof(bitmap_data_offset));
     tmp_u32 = header_size - 14;
-    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f); // sub header size
+    write(&tmp_u32, sizeof(tmp_u32)); // sub header size
     tmp_32 = bitmap->x;
-    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
+    write(&tmp_32, sizeof(tmp_32));
     tmp_32 = bitmap->y;
-    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
+    write(&tmp_32, sizeof(tmp_32));
 
     tmp_u16 = 1;
-    fwrite(&tmp_u16, sizeof(tmp_u16), 1, f); // color plane
-    fwrite(&n_pixel_bits, sizeof(n_pixel_bits), 1, f); // pixel depth
+    write(&tmp_u16, sizeof(tmp_u16)); // color plane
+    write(&n_pixel_bits, sizeof(n_pixel_bits)); // pixel depth
 
     tmp_u32 = 0;
-    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f); // compression method
+    write(&tmp_u32, sizeof(tmp_u32)); // compression method
 
     tmp_u32 = 0; //file_size - bitmap_data_offset;
-    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f); // image size
+    write(&tmp_u32, sizeof(tmp_u32)); // image size
     tmp_32 = 0;
-    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
-    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
+    write(&tmp_32, sizeof(tmp_32));
+    write(&tmp_32, sizeof(tmp_32));
     tmp_u32 = (!plt) ? 0 : plt->num_ents; // plt entries
-    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f);
+    write(&tmp_u32, sizeof(tmp_u32));
     tmp_u32 = 0;
-    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f);
+    write(&tmp_u32, sizeof(tmp_u32));
 
     if (plt) {
-        dump_palette(f, plt);
+        if (!dump_palette(f, plt))
+            goto write_err;
     }
     /* writing the data */
     for (i = 0; i < bitmap->data->num_chunks; i++) {
         SpiceChunk *chunk = &bitmap->data->chunk[i];
         int num_lines = chunk->len / bitmap->stride;
         for (j = 0; j < num_lines; j++) {
-            dump_line(f, chunk->data + (j * bitmap->stride), n_pixel_bits, bitmap->x, row_size);
+            if (!dump_line(f, chunk->data + (j * bitmap->stride), n_pixel_bits, bitmap->x, row_size))
+                goto write_err;
         }
     }
+
+write_err:
     fclose(f);
 }



Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]