[Spice-devel] [vdagent-win PATCH v10 4/6] Support encoding PNG images

[Uri Lublin]

On 07/24/2017 01:22 PM, Christophe Fergeau wrote:
> On Fri, Jul 21, 2017 at 02:51:39PM +0100, Frediano Ziglio wrote:
>> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
>> Acked-by: Christophe Fergeau <cfergeau at redhat.com>
>> ---
>>   vdagent/imagepng.cpp | 110 +++++++++++++++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 106 insertions(+), 4 deletions(-)
>>
>> diff --git a/vdagent/imagepng.cpp b/vdagent/imagepng.cpp
>> index 5642cc9..8a759f7 100644
>> --- a/vdagent/imagepng.cpp
>> +++ b/vdagent/imagepng.cpp
>> @@ -19,6 +19,7 @@
>>   
>>   #include <png.h>
>>   #include <algorithm>
>> +#include <vector>
>>   
>>   #include "imagepng.h"
>>   
>> @@ -36,9 +37,13 @@ private:
>>   struct BufferIo {
>>       uint8_t *buf;
>>       uint32_t pos, size;
>> +    bool allocated;
>>       BufferIo(uint8_t *_buf, uint32_t _size):
>> -        buf(_buf), pos(0), size(_size)
>> +        buf(_buf), pos(0), size(_size),
>> +        allocated(false)
>>       {}
>> +    ~BufferIo() { if (allocated) free(buf); }
>> +    uint8_t *release() { allocated = false; return buf; }
>>   };
>>   
>>   static void read_from_bufio(png_structp png, png_bytep out, png_size_t size)
>> @@ -50,6 +55,29 @@ static void read_from_bufio(png_structp png, png_bytep out, png_size_t size)
>>       io.pos += size;
>>   }
>>   
>> +static void write_to_bufio(png_structp png, png_bytep in, png_size_t size)
>> +{
>> +    BufferIo& io(*(BufferIo*)png_get_io_ptr(png));
>> +    if (io.pos + size > io.size) {
>> +        uint32_t new_size = io.size ? io.size * 2 : 4096;
>> +        while (io.pos + size >= new_size) {
>> +            new_size *= 2;
>> +        }
>> +        uint8_t *p = (uint8_t*) realloc(io.buf, new_size);
> 
> you are missing a if (!allocated) io.buf = NULL; here.

Why is that ? if (!allocated) then io.buf is not freed.

Note that man 3 realloc says:
"If realloc() fails, the original block is left untouched; it
  is not freed or moved."

Uri.

> (though I might
> try to use a std::vector in this codepath?)
> Christophe