Re: [PATCH v2 2/2] utils: fix endianess issues writing bitmap file and handle errors

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

 



On Thu, Jan 14, 2016 at 12:33:07PM +0000, Frediano Ziglio wrote:
> Do not print directly binary data but serialize in a portable way
> and then use write functions.
> Also handle errors writing to file.

Would have been nicer to have 2 separate patches for this.

Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>

> ---
>  server/spice-bitmap-utils.c | 106 +++++++++++++++++++++++++++-----------------
>  1 file changed, 66 insertions(+), 40 deletions(-)
> 
> diff --git a/server/spice-bitmap-utils.c b/server/spice-bitmap-utils.c
> index e6be15f..06b3a78 100644
> --- a/server/spice-bitmap-utils.c
> +++ b/server/spice-bitmap-utils.c
> @@ -137,23 +137,52 @@ int spice_bitmap_from_surface_type(uint32_t surface_format)
>  
>  #define RAM_PATH "/tmp/tmpfs"
>  
> -static void dump_palette(FILE *f, SpicePalette* plt)
> +static void put_16le(uint8_t **ptr, uint16_t val)
> +{
> +    **ptr = val & 0xffu;
> +    (*ptr)++;
> +    val >>= 8;
> +    **ptr = val & 0xffu;
> +    (*ptr)++;
> +}
> +
> +static void put_32le(uint8_t **ptr, uint32_t val)
> +{
> +    put_16le(ptr, val & 0xffffu);
> +    val >>= 16;
> +    put_16le(ptr, val & 0xffffu);
> +}
> +
> +#define WRITE(buf, size, f) 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), f);
>      }
> +    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)
>  {
>      static char zeroes[4] = { 0 };
>      int copy_bytes_size = SPICE_ALIGN(n_pixel_bits * width, 8) / 8;
>  
> -    fwrite(line, 1, copy_bytes_size, f);
> +    WRITE(line, copy_bytes_size, f);
>      // each line should be 4 bytes aligned
>      spice_assert(row_size - copy_bytes_size >= 0 && row_size - copy_bytes_size <= 4);
> -    fwrite(zeroes, 1, row_size - copy_bytes_size, f);
> +    WRITE(zeroes, row_size - copy_bytes_size, f);
> +    return true;
> +
> +write_err:
> +    return false;
>  }
>  
>  void dump_bitmap(SpiceBitmap *bitmap)
> @@ -170,11 +199,9 @@ void dump_bitmap(SpiceBitmap *bitmap)
>      int alpha = 0;
>      uint32_t header_size = 14 + 40;
>      uint32_t bitmap_data_offset;
> -    uint32_t tmp_u32;
> -    int32_t tmp_32;
> -    uint16_t tmp_u16;
>      FILE *f;
>      int i, j;
> +    uint8_t header[128], *ptr;
>  
>      switch (bitmap->format) {
>      case SPICE_BITMAP_FMT_1BIT_BE:
> @@ -233,47 +260,46 @@ void dump_bitmap(SpiceBitmap *bitmap)
>      }
>  
>      /* writing the bmp v3 header */
> -    fprintf(f, "BM");
> -    fwrite(&file_size, sizeof(file_size), 1, f);
> -    tmp_u16 = alpha ? 1 : 0;
> -    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);
> -    tmp_u32 = header_size - 14;
> -    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f); // sub header size
> -    tmp_32 = bitmap->x;
> -    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
> -    tmp_32 = bitmap->y;
> -    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
> -
> -    tmp_u32 = 0;
> -    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
> -    tmp_32 = 0;
> -    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
> -    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);
> -    tmp_u32 = 0;
> -    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f);
> +    ptr = header;
> +    put_16le(&ptr, 0x4D42); // "BM"
> +    put_32le(&ptr, file_size);
> +    put_16le(&ptr, alpha);
> +    put_16le(&ptr, 0);
> +    put_32le(&ptr, bitmap_data_offset);
> +    put_32le(&ptr, header_size - 14);
> +    put_32le(&ptr, bitmap->x);
> +    put_32le(&ptr, bitmap->y);
> +
> +    put_16le(&ptr, 1); // plane
> +    put_16le(&ptr, n_pixel_bits);
> +
> +    put_32le(&ptr, 0); // compression
> +
> +    put_32le(&ptr, 0); //file_size - bitmap_data_offset;
> +    put_32le(&ptr, 0);
> +    put_32le(&ptr, 0);
> +    put_32le(&ptr, !plt ? 0 : plt->num_ents); // plt entries
> +    put_32le(&ptr, 0);
> +
> +    WRITE(header, ptr - header, f);
>  
>      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;
>          }
>      }
>      fclose(f);
> +    return;
> +
> +write_err:
> +    fclose(f);
> +    remove(file_str);
>  }
> -- 
> 2.4.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel

Attachment: signature.asc
Description: PGP signature

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