In addition to the (mostly) nits below, this seems ripe for some unit
tests. Especially for the overrun cases and the data alignment cases.
On 12/12/2014 02:43 PM, Carl Worth 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.
> ---
>
> Jason,
>
> This second revision of my patch is in response to your review.
>
> I went to move the align_blob_reader function down by all the other
> blob_reader functions, but I decided not to. You had a concern about the
> alignment code, and it it's wrong then both align_blob() and
> align_blob_reader() will need to be fixed. So it seems useful to me to keep
> them close to each other.
>
> -Carl
>
> src/glsl/Makefile.sources | 3 +-
> src/glsl/blob.c | 295
> ++++++++++++++++++++++++++++++++++++++++++++++
> src/glsl/blob.h | 245 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 542 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
Alphabetize
>
> # glsl_compiler
>
> diff --git a/src/glsl/blob.c b/src/glsl/blob.c
> new file mode 100644
> index 0000000..75d3cda
> --- /dev/null
> +++ b/src/glsl/blob.c
> @@ -0,0 +1,295 @@
> +/*
> + * 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)
^ No space here or other similar places.
> +{
> + 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;
Can additional ever be > BLOB_INITIAL_SIZE?
> + else
> + to_allocate = MAX2(blob->allocated * 2, additional);
> +
> + 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);
const size_t new_size = ALIGN(blob->size, alignment);
> +
> + if (! grow_to_fit (blob, new_size - blob->size))
^ No space here or other similar places.
> + 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);
> +}
> +
> +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)
> +{
> + 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 (! ensure_can_read (blob, size))
> + return NULL;
> +
> + 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 we're already at the end, then this is an overrun. */
> + if (blob->current >= blob->end) {
> + blob->overrun = true;
> + return NULL;
> + }
> +
> + /* Similarly, if there is no zero byte in the data remaining in this blob,
> + * we also consider that an overrun. */
*/ should be on its own line for a multiline comment.
> + nul = memchr (blob->current, 0, blob->end - blob->current);
> +
> + if (nul == NULL) {
> + blob->overrun = true;
> + return NULL;
> + }
> +
> + size = nul - blob->current + 1;
> +
> + assert (ensure_can_read(blob, size));
> +
> + 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..8d0d959
> --- /dev/null
> +++ b/src/glsl/blob.h
> @@ -0,0 +1,245 @@
> +/*
> + * 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.
> + */
These comments should be inside the structure.
> +struct blob {
> + uint8_t *data;
/** Number of bytes that have actually been allocated for \c data. */
> + size_t allocated;
/** Number of bytes used to store data. */
> + 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).
blob->data or blob->current?
> + *
> + * 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);
> +
> +/**
> + * Add a uint32_t to a blob.
> + *
> + * Note: This function will only write to a uint32_t-aligned offset from the
Use \note
> + * 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
\note
> + * 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).
initializing
> + *
> + * 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 */
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
