On Wednesday, February 11, 2015 04:56:56 PM Juha-Pekka Heikkila wrote:
> There is no error path available thus instead of giving
> realloc possibility to fail use new which will never
> return null pointer and throws bad_alloc on failure.
>
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikk...@gmail.com>
> ---
> src/mesa/drivers/dri/i965/brw_ir_allocator.h | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_ir_allocator.h
> b/src/mesa/drivers/dri/i965/brw_ir_allocator.h
> index b1237ed..5330bff 100644
> --- a/src/mesa/drivers/dri/i965/brw_ir_allocator.h
> +++ b/src/mesa/drivers/dri/i965/brw_ir_allocator.h
> @@ -40,8 +40,8 @@ namespace brw {
>
> ~simple_allocator()
> {
> - free(offsets);
> - free(sizes);
> + delete[] offsets;
> + delete[] sizes;
> }
>
> unsigned
> @@ -49,8 +49,16 @@ namespace brw {
> {
> if (capacity <= count) {
> capacity = MAX2(16, capacity * 2);
> - sizes = (unsigned *)realloc(sizes, capacity * sizeof(unsigned));
> - offsets = (unsigned *)realloc(offsets, capacity *
> sizeof(unsigned));
> +
> + unsigned *tmp_sizes = new unsigned[capacity];
> + memcpy(tmp_sizes, sizes, count * sizeof(unsigned));
> + delete[] sizes;
> + sizes = tmp_sizes;
> +
> + unsigned *tmp_offsets = new unsigned[capacity];
> + memcpy(tmp_offsets, offsets, count * sizeof(unsigned));
> + delete[] offsets;
> + offsets = tmp_offsets;
> }
>
> sizes[count] = size;
> Okay, I'll go ahead and say what Matt and Jason were probably thinking: NAK. I'm not against using new/delete in general, but using realloc to double the size of arrays is a pattern we use all over the codebase. I don't see any reason not to use that same pattern here. Plus, the old code was 2 lines of code...and now it's 8 lines of code, for no real benefit. Most idiomatic C++ code I've seen would just some kind of flexible array class, like std::vector, rather than open coding this. So, we're already divergent from C++ best practices, and instead are following the typical C idiom. This is neither. In fact, your new code is more likely to fail than the old one: instead of growing the array from size N to size 2*N---which realloc may be able to do in-place---you're briefly keeping both around, using N+2*N=3*N space. If realloc failed, this would absolutely fail too. If we compiled without exceptions, this would crash in an identical manner - no actual error handling is done here. With exceptions, it's not much better - we throw an exception all the way outside of libGL and back to the application (since we sure don't handle those), leaving our work in some unknown state. Plus, as has been repeatedly mentioned - malloc doesn't fail if you're out of memory - it only fails if you're out of virtual address space. --Ken
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
