[Mesa-dev] [PATCH] i965: Use new/delete instead of realloc() in brw_ir_allocator.h

[Matt Turner]

On Wed, Feb 11, 2015 at 9:35 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On Wed, Feb 11, 2015 at 12:30 PM, Francisco Jerez <currojerez at riseup.net>
> wrote:
>>
>> Matt Turner <mattst88 at gmail.com> writes:
>>
>> > On Wed, Feb 11, 2015 at 9:16 AM, Francisco Jerez <currojerez at riseup.net>
>> > wrote:
>> >> Matt Turner <mattst88 at gmail.com> writes:
>> >>
>> >>> On Wed, Feb 11, 2015 at 6:37 AM, Juha-Pekka Heikkila
>> >>> <juhapekka.heikkila at gmail.com> 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.
>> >>>
>> >>> The problem was that we weren't checking if realloc failed.
>> >>>
>> >>> Why is the solution to change from malloc/free to new/delete?
>> >>
>> >> The new operator is guaranteed not to return NULL by the C++ standard.
>> >> Aside from that Juha-Pekka's code seems more idiomatic to me than
>> >> calling realloc() from a C++ source file, but that might just be a
>> >> matter of taste.
>> >
>> > But new will throw an exception if it fails, right? Presumably under
>> > the same conditions as malloc/realloc returning NULL (i.e., being out
>> > of address space).
>>
>> Yeah, so this patch doesn't really handle the out of memory condition.
>> According to Juha-Pekka it silences a Klocwork warning and IMHO it
>> improves code-style slightly.  But, sure, if it actually happens it's
>> just trading one kind of failure for another.
>
>
> The fact of the matter is that we don't handle exceptions either so we get a
> crash either way.  It's just the difference between a segfault or an
> unhandled exception.  Also, for whatever it's worth, in the case where
> realloc can expand in-place, the realloc call will be faster because we
> won't have to do a memcpy.

Indeed. And another thing to consider is that we've discussed
compiling with -fno-exceptions.

I think it's okay to add out-of-memory checks where we can reasonably
do something about them, but code churn *just* to silence a tool (one
that I'm not aware of catching any meaningful bugs) doesn't seem good.