Hey Carl, Yeah, this looks nifty. I've got some comments on the implementation though. On Dec 11, 2014 11:55 AM, "Carl Worth" <<a href="mailto:cworth@cworth.org">cworth@cworth.org</a>> wrote: > > This new interface allows for writing a series of objects to a chunk > of memory (a "blob").. The allocated memory is maintained within the > blob itself, (and re-allocated by doubling when necessary). > > There are also functions for reading objects from a blob as well. If > code attempts to read beyond the available memory, the read functions > return 0 values (or its moral equivalent) without reading past the > allocated memory. Once the caller is done with the reads, it can check > blob->overrun to ensure whether any invalid values were previously > returned due to attempts to read too far. > --- > src/glsl/Makefile.sources | 3 +- > src/glsl/blob.c | 287 ++++++++++++++++++++++++++++++++++++++++++++++ > src/glsl/blob.h | 263 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 552 insertions(+), 1 deletion(-) > create mode 100644 src/glsl/blob.c > create mode 100644 src/glsl/blob.h > > diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources > index 676fa0d..875e4bf 100644 > --- a/src/glsl/Makefile.sources > +++ b/src/glsl/Makefile.sources > @@ -105,7 +105,8 @@ LIBGLSL_FILES = \ > $(GLSL_SRCDIR)/opt_swizzle_swizzle.cpp \ > $(GLSL_SRCDIR)/opt_tree_grafting.cpp \ > $(GLSL_SRCDIR)/opt_vectorize.cpp \ > - $(GLSL_SRCDIR)/s_expression.cpp > + $(GLSL_SRCDIR)/s_expression.cpp \ > + $(GLSL_SRCDIR)/blob.c > > # glsl_compiler > > diff --git a/src/glsl/blob.c b/src/glsl/blob.c > new file mode 100644 > index 0000000..0af71fc > --- /dev/null > +++ b/src/glsl/blob.c > @@ -0,0 +1,287 @@ > +/* > + * Copyright © 2014 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. > + */ > + > +#include <string.h> > + > +#include "main/macros.h" > +#include "util/ralloc.h" > +#include "blob.h" > + > +#define BLOB_INITIAL_SIZE 4096 > + > +/* Ensure that \blob will be able to fit an additional object of size > + * \additional. The growing (if any) will occur by doubling the existing > + * allocation. > + */ > +static bool > +grow_to_fit (struct blob *blob, size_t additional) > +{ > + size_t to_allocate; > + uint8_t *new_data; > + > + if (blob->size + additional <= blob->allocated) > + return true; > + > + if (blob->allocated == 0) > + to_allocate = BLOB_INITIAL_SIZE; > + else > + to_allocate = blob->allocated * 2; What happens if additional is bigger than blob->size? Maybe iterate (if you want to keep power-of-two) or take a max here. > + > + new_data = reralloc_size(blob, blob->data, to_allocate); > + if (new_data == NULL) > + return false; > + > + blob->data = new_data; > + blob->allocated = to_allocate; > + > + return true; > +} > + > +/* Align the blob->size so that reading or writing a value at (blob->data + > + * blob->size) will result in an access aligned to a granularity of \alignment > + * bytes. > + * > + * \return True unless allocation fails > + */ > +static bool > +align_blob (struct blob *blob, size_t alignment) > +{ > + size_t new_size; > + > + new_size = ALIGN(blob->size, alignment); Do we what to align the pointer or the size? Aligning the size only makes sense if the pointer is already aligned. At the very least, we should assert that the requested alignment is less than or equal to the known pointer alignment for the start of the blob. > + > + if (! grow_to_fit (blob, new_size - blob->size)) > + return false; > + > + blob->size = new_size; > + > + return true; > +} > + > +static void > +align_blob_reader (struct blob_reader *blob, size_t alignment) > +{ > + blob->current = blob->data + ALIGN(blob->current - blob->data, alignment); Again, pointer or size? Also, why is this way up here when the other reader stuff comes later. > +} > + > +struct blob * > +blob_create (void *mem_ctx) > +{ > + struct blob *blob; > + > + blob = ralloc(mem_ctx, struct blob); > + if (blob == NULL) > + return NULL; > + > + blob->data = NULL; > + blob->allocated = 0; > + blob->size = 0; > + > + return blob; > +} > + > +bool > +blob_write_bytes (struct blob *blob, const void *bytes, size_t to_write) > +{ > + if (! grow_to_fit (blob, to_write)) > + return false; > + > + memcpy (blob->data + blob->size, bytes, to_write); > + blob->size += to_write; > + > + return true; > +} > + > +uint8_t * > +blob_reserve_bytes (struct blob *blob, size_t to_write) > +{ > + uint8_t *ret; > + > + if (! grow_to_fit (blob, to_write)) > + return NULL; > + > + ret = blob->data + blob->size; > + blob->size += to_write; > + > + return ret; > +} > + > +bool > +blob_write_uint32 (struct blob *blob, uint32_t value) > +{ > + align_blob(blob, sizeof(value)); > + > + return blob_write_bytes(blob, &value, sizeof(value)); > +} > + > +bool > +blob_write_uint64 (struct blob *blob, uint64_t value) > +{ > + align_blob(blob, sizeof(value)); > + > + return blob_write_bytes(blob, &value, sizeof(value)); > +} > + > +bool > +blob_write_intptr (struct blob *blob, intptr_t value) > +{ > + align_blob(blob, sizeof(value)); > + > + return blob_write_bytes(blob, &value, sizeof(value)); > +} > + > +bool > +blob_write_string (struct blob *blob, const char *str) > +{ > + return blob_write_bytes(blob, str, strlen(str) + 1); > +} > + > +void > +blob_reader_init (struct blob_reader *blob, uint8_t *data, size_t size) Calling it blob is a bit disconcerting, but probably not a big deal. > +{ > + blob->data = data; > + blob->end = data + size; > + blob->current = data; > + blob->overrun = false; > +} > + > +/* Check that an object of size \size can be read from this blob. > + * > + * If not, set blob->overrun to indicate that we attempted to read too far. > + */ > +static bool > +ensure_can_read (struct blob_reader *blob, size_t size) > +{ > + if (blob->current < blob->end && blob->end - blob->current >= size) > + return true; > + > + blob->overrun = true; > + > + return false; > +} > + > +void * > +blob_read_bytes (struct blob_reader *blob, size_t size) > +{ > + void *ret; > + > + if (blob->current > blob->end || blob->end - blob->current < size) > + return NULL; Why isn't this a call to ensure_can_read? Without this, the overrun flag never gets set. We also don't get overrun set for out-of-bounds copies and string reads. If that's intentional, it should be documented. Also, you reversed the logic wrong. It should be current >= end. > + > + ret = blob->current; > + > + blob->current += size; > + > + return ret; > +} > + > +void > +blob_copy_bytes (struct blob_reader *blob, uint8_t *dest, size_t size) > +{ > + uint8_t *bytes; > + > + bytes = blob_read_bytes (blob, size); > + if (bytes == NULL) > + return; > + > + memcpy (dest, bytes, size); > +} > + > +uint32_t > +blob_read_uint32 (struct blob_reader *blob) > +{ > + uint32_t ret; > + int size = sizeof(ret); > + > + align_blob_reader(blob, size); > + > + if (! ensure_can_read(blob, size)) > + return 0; > + > + ret = *((uint32_t*) blob->current); > + > + blob->current += size; > + > + return ret; > +} > + > +uint64_t > +blob_read_uint64 (struct blob_reader *blob) > +{ > + uint64_t ret; > + int size = sizeof(ret); > + > + align_blob_reader(blob, size); > + > + if (! ensure_can_read(blob, size)) > + return 0; > + > + ret = *((uint64_t*) blob->current); > + > + blob->current += size; > + > + return ret; > +} > + > +intptr_t > +blob_read_intptr (struct blob_reader *blob) > +{ > + intptr_t ret; > + int size = sizeof(ret); > + > + align_blob_reader(blob, size); > + > + if (! ensure_can_read(blob, size)) > + return 0; > + > + ret = *((intptr_t *) blob->current); > + > + blob->current += size; > + > + return ret; > +} > + > +char * > +blob_read_string (struct blob_reader *blob) > +{ > + int size; > + char *ret; > + uint8_t *nul; > + > + /* If there is no NULL terminator, the reader gets nothing. */ > + nul = memchr (blob->current, 0, blob->end - blob->current); Should we check for current >= end first?... > + > + if (nul == NULL) > + return NULL; > + > + size = nul - blob->current + 1; > + > + if (! ensure_can_read(blob, size)) > + return 0; ...If we do, this does nothing. We may want to set the overrun flag if there is no null. > + > + ret = (char *) blob->current; > + > + blob->current += size; > + > + return ret; > +} > diff --git a/src/glsl/blob.h b/src/glsl/blob.h > new file mode 100644 > index 0000000..374b21e > --- /dev/null > +++ b/src/glsl/blob.h > @@ -0,0 +1,263 @@ > +/* > + * Copyright © 2014 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 BLOB_H > +#define BLOB_H > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +#include <stdint.h> > + > +/* The blob functions implement a simple, low-level API for serializing and > + * deserializing. > + * > + * All objects written to a blob will be serialized directly, (without any > + * additional meta-data to describe the data written). Therefore, it is the > + * caller's responsibility to ensure that any data can be read later, (either > + * by knowing exactly what data is expected, or by writing to the blob > + * sufficient meta-data to describe what has been written). > + * > + * A blob is efficient in that it dynamically grows by doubling in size, so > + * allocation costs are logarithmic. > + */ > + > +/* The allocated size is the number of bytes that have actually been allocated > + * for the "data" array, while "size" is the number of bytes actually used to > + * store data. > + */ > +struct blob { > + uint8_t *data; > + size_t allocated; > + size_t size; > +}; > + > +/* When done reading, the caller can ensure that everything was consumed by > + * checking the following: > + * > + * 1. blob->data should be equal to blob->end, (if not, too little was > + * read). > + * > + * 2. blob->overrun should be false, (otherwise, too much was read). > + */ > +struct blob_reader { > + uint8_t *data; > + uint8_t *end; > + uint8_t *current; > + bool overrun; > +}; > + > +/** > + * Create a new, empty blob, belonging to \mem_ctx. > + * > + * \return The new blob, (or NULL in case of allocation failure). > + */ > +struct blob * > +blob_create (void *mem_ctx); > + > +/** > + * Add some unstructured, fixed-size data to a blob. > + * > + * \return True unless allocation failed. > + */ > +bool > +blob_write_bytes (struct blob *blob, const void *bytes, size_t to_write); > + > +/** > + * Reserve space in \blob for a number of bytes. > + * > + * Space will be allocated within the blob for these byes, but the bytes will > + * be left uninitialized. The caller is expected to use the return value to > + * write directly (and immediately) to these bytes. > + * > + * NOTE: The return value is valid immediately upon return, but can be > + * invalidated by any other call to a blob function. So the caller should call > + * blob_reserve_byes immediately before writing through the returned pointer. > + * > + * This function is intended to be used when interfacing with an existing API > + * that is not aware of the blob API, (so that blob_write_bytes cannot be > + * called). > + * > + * \return A pointer to space allocated within \blob to which \to_write bytes > + * can be written, (or NULL in case of any allocation error). > + */ > +uint8_t * > +blob_reserve_bytes (struct blob *blob, size_t to_write); > + > +/** > + * Overwrite some data previously written to the blob. > + * > + * Writes data to an existing portion of the blob at an offset of \offset. > + * This data range must have previously been written to the blob by one of the > + * blob_write_* calls. > + * > + * For example usage, see blob_overwrite_uint32 > + * > + * \return True unless the requested offset or offset+to_write lie outside > + * the current blob's size. > + */ > +bool > +blob_overwrite_bytes (struct blob *blob, > + size_t offset, > + const void *bytes, > + size_t to_write); This function isn't implemented. > + > +/** > + * Add a uint32_t to a blob. > + * > + * Note: This function will only write to a uint32_t-aligned offset from the > + * beginning of the blob's data, so some padding bytes may be added to the > + * blob if this write follows some unaligned write (such as > + * blob_write_string). > + * > + * \return True unless allocation failed. > + */ > +bool > +blob_write_uint32 (struct blob *blob, uint32_t value); > + > +/** > + * Add a uint64_t to a blob. > + * > + * Note: This function will only write to a uint64_t-aligned offset from the > + * beginning of the blob's data, so some padding bytes may be added to the > + * blob if this write follows some unaligned write (such as > + * blob_write_string). > + * > + * \return True unless allocation failed. > + */ > +bool > +blob_write_uint64 (struct blob *blob, uint64_t value); > + > +/** > + * Add an intptr_t to a blob. > + * > + * Note: This function will only write to an intptr_t-aligned offset from the > + * beginning of the blob's data, so some padding bytes may be added to the > + * blob if this write follows some unaligned write (such as > + * blob_write_string). > + * > + * \return True unless allocation failed. > + */ > +bool > +blob_write_intptr (struct blob *blob, intptr_t value); > + > +/** > + * Add a NULL-terminated string to a blob, (including the NULL terminator). > + * > + * \return True unless allocation failed. > + */ > +bool > +blob_write_string (struct blob *blob, const char *str); > + > +/** > + * Start reading a blob, (inintializing the contents of \blob for reading). > + * > + * After this call, the caller can use the various blob_read_* functions to > + * read elements from the data array. > + * > + * For all of the blob_read_* functions, if there is insufficient data > + * remaining, the functions will do nothing, (perhaps returning default values > + * such as 0). The caller can detect this by noting that the blob_reader's > + * current value is unchanged before and after the call. > + */ > +void > +blob_reader_init (struct blob_reader *blob, uint8_t *data, size_t size); > + > +/** > + * Read some unstructured, fixed-size data from the current location, (and > + * update the current location to just past this data). > + * > + * The memory returned belongs to the data underlying the blob reader. The > + * caller must copy the data in order to use it after the lifetime of the data > + * underlying the blob reader. > + * > + * \return The bytes read (see note above about memory lifetime). > + */ > +void * > +blob_read_bytes (struct blob_reader *blob, size_t size); > + > +/** > + * Read some unstructured, fixed-size data from the current location, copying > + * it to \dest (and update the current location to just past this data) > + */ > +void > +blob_copy_bytes (struct blob_reader *blob, uint8_t *dest, size_t size); > + > +/** > + * Read a uint32_t from the current location, (and update the current location > + * to just past this uint32_t). > + * > + * Note: This function will only read from a uint32_t-aligned offset from the > + * beginning of the blob's data, so some padding bytes may be skipped. > + * > + * \return The uint32_t read > + */ > +uint32_t > +blob_read_uint32 (struct blob_reader *blob); > + > +/** > + * Read a uint64_t from the current location, (and update the current location > + * to just past this uint64_t). > + * > + * Note: This function will only read from a uint64_t-aligned offset from the > + * beginning of the blob's data, so some padding bytes may be skipped. > + * > + * \return The uint64_t read > + */ > +uint64_t > +blob_read_uint64 (struct blob_reader *blob); > + > +/** > + * Read an intptr_t value from the current location, (and update the > + * current location to just past this intptr_t). > + * > + * Note: This function will only read from an intptr_t-aligned offset from the > + * beginning of the blob's data, so some padding bytes may be skipped. > + * > + * \return The intptr_t read > + */ > +intptr_t > +blob_read_intptr (struct blob_reader *blob); > + > +/** > + * Read a NULL-terminated string from the current location, (and update the > + * current location to just past this string). > + * > + * The memory returned belongs to the data underlying the blob reader. The > + * caller must copy the string in order to use the string after the lifetime > + * of the data underlying the blob reader. > + * > + * \return The string read (see note above about memory lifetime). However, if > + * there is no NULL byte remaining within the blob, this function returns > + * NULL. > + */ > +char * > +blob_read_string (struct blob_reader *blob); > + > +#ifdef __cplusplus > +} > +#endif > + > +#endif /* BLOB_H */ > -- > 2.1.1 > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>