[Mesa-dev] [wip 1/9] glsl: memory_writer helper class for data serialization

[Tapani Pälli]

On 01/13/2014 08:29 PM, Ian Romanick wrote:
> On 01/02/2014 03:58 AM, Tapani Pälli wrote:
>> Class will be used by the shader binary cache implementation.
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/glsl/memory_writer.h | 147 +++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 147 insertions(+)
>>   create mode 100644 src/glsl/memory_writer.h
>>
>> diff --git a/src/glsl/memory_writer.h b/src/glsl/memory_writer.h
>> new file mode 100644
>> index 0000000..a6c6b55
>> --- /dev/null
>> +++ b/src/glsl/memory_writer.h
>> @@ -0,0 +1,147 @@
>> +/* -*- c++ -*- */
>> +/*
>> + * Copyright © 2013 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> + * DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +#pragma once
>> +#ifndef MEMORY_WRITER_H
>> +#define MEMORY_WRITER_H
>> +
>> +#include <stdlib.h>
>> +#include <unistd.h>
>> +#include <string.h>
>> +
>> +#ifdef __cplusplus
>> +/**
>> + * Helper class for writing data to memory
>> + *
>> + * This class maintains a dynamically-sized memory buffer and allows
>> + * for data to be efficiently appended to it with automatic resizing.
>> + */
>> +class memory_writer
>> +{
>> +public:
>> +   memory_writer() :
>> +      memory(NULL),
>> +      curr_size(0),
>> +      pos(0) {}
>> +
>> +   ~memory_writer()
>> +   {
>> +      free(memory);
>> +   }
>> +
>> +   /* user wants to claim the memory */
>> +   char *release_memory(size_t *size)
>> +   {
>> +      /* final realloc to free allocated but unused memory */
>> +      char *result = (char *) realloc(memory, pos);
>> +      *size = pos;
>> +      memory = NULL;
>> +      curr_size = 0;
>> +      pos = 0;
>> +      return result;
>> +   }
>> +
>> +/**
>> + * write functions per type
>> + */
>> +#define DECL_WRITER(type) int write_ ##type (const type data) {\
>> +   return write(&data, sizeof(type));\
>> +}
>> +
>> +   DECL_WRITER(int32_t);
>> +   DECL_WRITER(int64_t);
>> +   DECL_WRITER(uint8_t);
>> +   DECL_WRITER(uint32_t);
>> +
>> +   int write_bool(bool data)
> I agree with Paul's previous comments about the return values.
>
> http://lists.freedesktop.org/archives/mesa-dev/2013-November/047740.html
>
> It looks like the only errors tested are either memory allocation or bad
> parameters.  The bad parameter checks should just be assertions.

OK, I'll remove error checking from here for now as it is not really 
used by the serialization.

>> +   {
>> +      uint8_t val = data;
>> +      return write_uint8_t(val);
>> +   }
>> +
>> +   /* write function that reallocates more memory if required */
>> +   int write(const void *data, int32_t size)
>> +   {
>> +      if (!memory || pos > (int32_t)(curr_size - size))
>> +         if (grow(size))
>> +            return -1;
>> +
>> +      memcpy(memory + pos, data, size);
>> +
>> +      pos += size;
>> +      return 0;
>> +   }
>> +
>> +   int overwrite(const void *data, int32_t size, int32_t offset)
>> +   {
>> +      if (offset < 0 || offset + size > pos)
>> +         return -1;
>> +      memcpy(memory + offset, data, size);
>> +      return 0;
>> +   }
>> +
>> +   int write_string(const char *str)
>> +   {
>> +      if (!str)
>> +         return -1;
>> +      char terminator = '\0';
>> +      write(str, strlen(str));
>> +      write(&terminator, 1);
> This should just be
>
>        write(str, strlen(str) + 1);

fixed

>> +      return 0;
>> +   }
>> +
>> +   inline int32_t position() { return pos; }
>> +
>> +
>> +private:
>> +
>> +   /* reallocate more memory */
>> +   int grow(int32_t size)
>> +   {
>> +      int32_t new_size = 2 * (curr_size + size);
>> +      char *more_mem = (char *) realloc(memory, new_size);
>> +      if (more_mem == NULL) {
>> +         free(memory);
>> +         memory = NULL;
>> +         return -1;
>> +      } else {
>> +         memory = more_mem;
>> +         curr_size = new_size;
>> +         return 0;
>> +      }
>> +   }
>> +
>> +   /* allocated memory */
>> +   char *memory;
>> +
>> +   /* current size of the whole allocation */
>> +   int32_t curr_size;
> Is there a reason to specifically make this int32_t instead of just int?
>   Or even unsigned?

Not really, can be changed.

>> +
>> +   /* write position / size of the data written */
>> +   int32_t pos;
>> +};
>> +
>> +#endif /* ifdef __cplusplus */
>> +
>> +#endif /* MEMORY_WRITER_H */
>>

// Tapani