Re: Replace OCTAVE_LOCAL_BUFFER implementation with std::unique_ptr?

[Rik]

On 07/19/2017 11:24 AM, John W. Eaton wrote:
> On 07/19/2017 11:25 AM, Rik wrote:
>
>> It wasn't that hard so I made two different implementations of
>> OCTAVE_LOCAL_BUFFER. One based on bytes, and one based on the passed in
>> object type T. Unfortunately, while they compile just fine, the
>> resulting binary segfaults. I think what this really means is that there
>> are issues in Octave code around double frees of memory.  I've put my
>> work up on the bug report you mentioned.
>
> I also saw a crash on startup with your change.  I think you want to use
> something like
>
> #define OCTAVE_LOCAL_BUFFER(T, buf, size)                               \
>   std::unique_ptr<T []> octave_local_buffer_ ## buf { new T [size] };   \
>   T *buf = octave_local_buffer_ ## buf.get ()
>
> for arrays.  I posted an updated patch to the bug tracker.  Using that
> patch, I uncovered another memory error that I think is fixed with this
> changeset:
>
>   http://hg.savannah.gnu.org/hgweb/octave/rev/99a9e19bae41
>
> With that, the test suite runs as before.
>

I tested several different initialization strategies including default,
aggregate, and value initialization and there was not much difference.  I
used default initialization in the end (what is represented by the code
above) because there didn't seem to be a need to zero-initialize POD types
in a temporary buffer.

Also, while looking through the code I found instances where
OCTAVE_LOCAL_BUFFER was being used with constants that are small and fixed
at compile time.  Given that OCTAVE_LOCAL_BUFFER is designed for dynamic,
and possibly large, buffers which are stored on the heap, I believe it
makes sense to replace these instances with local variables which would be
stored on the stack.  As an example,

corefcn/load-save.cc:225:  OCTAVE_LOCAL_BUFFER (unsigned char, magic, 2);

A 2-byte scratchpad just isn't worth the new/delete overhead.

Am I missing something, or should I go ahead and convert these static
buffers to local variables?

--Rik