> > 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