Re: [Mesa-dev] [PATCH 2/3] u_dynarray: return 0 on realloc failure
This is basically the same issue as with patch #3... it's not so clear
what the best policy actually is.
Cheers,
Nicolai
On 05.05.19 01:24, Caio Marcelo de Oliveira Filho wrote:
> Hi,
>
>>> diff --git a/src/util/u_dynarray.h b/src/util/u_dynarray.h
>>> index b30fd7b1154..f6a81609dbe 100644
>>> --- a/src/util/u_dynarray.h
>>> +++ b/src/util/u_dynarray.h
>>> @@ -85,20 +85,22 @@ util_dynarray_ensure_cap(struct util_dynarray *buf,
>>> unsigned newcap)
>>>buf->capacity = DYN_ARRAY_INITIAL_SIZE;
>>>
>>> while (newcap > buf->capacity)
>>>buf->capacity *= 2;
>>>
>>> if (buf->mem_ctx) {
>>>buf->data = reralloc_size(buf->mem_ctx, buf->data,
>>> buf->capacity);
>>> } else {
>>>buf->data = realloc(buf->data, buf->capacity);
>>> }
>>> + if (!buf->data)
>>> + return 0;
>>
>> To keep buf->data valid, put the new value in a temporary variable and
>> copy it into buf->data on success. If realloc and reralloc_size fail,
>> the original pointer is still valid, while if we overwrite buf->data
>> we are guaranteed to leak the data on failure.
>
> You also want to use a temporary variable for capacity. If realloc
> fails and we keep the old data, we also want to keep the old capacity.
>
>
> Caio
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/3] u_dynarray: return 0 on realloc failure
Hi,
> > diff --git a/src/util/u_dynarray.h b/src/util/u_dynarray.h
> > index b30fd7b1154..f6a81609dbe 100644
> > --- a/src/util/u_dynarray.h
> > +++ b/src/util/u_dynarray.h
> > @@ -85,20 +85,22 @@ util_dynarray_ensure_cap(struct util_dynarray *buf,
> > unsigned newcap)
> > buf->capacity = DYN_ARRAY_INITIAL_SIZE;
> >
> >while (newcap > buf->capacity)
> > buf->capacity *= 2;
> >
> >if (buf->mem_ctx) {
> > buf->data = reralloc_size(buf->mem_ctx, buf->data, buf->capacity);
> >} else {
> > buf->data = realloc(buf->data, buf->capacity);
> >}
> > + if (!buf->data)
> > + return 0;
>
> To keep buf->data valid, put the new value in a temporary variable and
> copy it into buf->data on success. If realloc and reralloc_size fail,
> the original pointer is still valid, while if we overwrite buf->data
> we are guaranteed to leak the data on failure.
You also want to use a temporary variable for capacity. If realloc
fails and we keep the old data, we also want to keep the old capacity.
Caio
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/3] u_dynarray: return 0 on realloc failure
On Sat, May 4, 2019 at 3:25 PM Nicolai Hähnle wrote:
>
> From: Nicolai Hähnle
>
> We're not very good at handling out-of-memory conditions in general, but
> this change at least gives the caller the option of handling it.
>
> This happens to fix an error in out-of-memory handling in i965, which has
> the following code in brw_bufmgr.c:
>
> node = util_dynarray_grow(vma_list, sizeof(struct vma_bucket_node));
> if (unlikely(!node))
> return 0ull;
>
> Previously, allocation failure for util_dynarray_grow wouldn't actually
> return NULL when the dynarray was previously non-empty.
> ---
> src/util/u_dynarray.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/src/util/u_dynarray.h b/src/util/u_dynarray.h
> index b30fd7b1154..f6a81609dbe 100644
> --- a/src/util/u_dynarray.h
> +++ b/src/util/u_dynarray.h
> @@ -85,20 +85,22 @@ util_dynarray_ensure_cap(struct util_dynarray *buf,
> unsigned newcap)
> buf->capacity = DYN_ARRAY_INITIAL_SIZE;
>
>while (newcap > buf->capacity)
> buf->capacity *= 2;
>
>if (buf->mem_ctx) {
> buf->data = reralloc_size(buf->mem_ctx, buf->data, buf->capacity);
>} else {
> buf->data = realloc(buf->data, buf->capacity);
>}
> + if (!buf->data)
> + return 0;
To keep buf->data valid, put the new value in a temporary variable and
copy it into buf->data on success. If realloc and reralloc_size fail,
the original pointer is still valid, while if we overwrite buf->data
we are guaranteed to leak the data on failure.
> }
>
> return (void *)((char *)buf->data + buf->size);
> }
>
> static inline void *
> util_dynarray_grow_cap(struct util_dynarray *buf, int diff)
> {
> return util_dynarray_ensure_cap(buf, buf->size + diff);
> }
> --
> 2.20.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] u_dynarray: return 0 on realloc failure
From: Nicolai Hähnle
We're not very good at handling out-of-memory conditions in general, but
this change at least gives the caller the option of handling it.
This happens to fix an error in out-of-memory handling in i965, which has
the following code in brw_bufmgr.c:
node = util_dynarray_grow(vma_list, sizeof(struct vma_bucket_node));
if (unlikely(!node))
return 0ull;
Previously, allocation failure for util_dynarray_grow wouldn't actually
return NULL when the dynarray was previously non-empty.
---
src/util/u_dynarray.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/util/u_dynarray.h b/src/util/u_dynarray.h
index b30fd7b1154..f6a81609dbe 100644
--- a/src/util/u_dynarray.h
+++ b/src/util/u_dynarray.h
@@ -85,20 +85,22 @@ util_dynarray_ensure_cap(struct util_dynarray *buf,
unsigned newcap)
buf->capacity = DYN_ARRAY_INITIAL_SIZE;
while (newcap > buf->capacity)
buf->capacity *= 2;
if (buf->mem_ctx) {
buf->data = reralloc_size(buf->mem_ctx, buf->data, buf->capacity);
} else {
buf->data = realloc(buf->data, buf->capacity);
}
+ if (!buf->data)
+ return 0;
}
return (void *)((char *)buf->data + buf->size);
}
static inline void *
util_dynarray_grow_cap(struct util_dynarray *buf, int diff)
{
return util_dynarray_ensure_cap(buf, buf->size + diff);
}
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
