https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119323

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #2)
> More important than the missing self-assignment check and the missing copy
> constructor, there's no destructor.
> 
> The missing check and copy ctor only matter if the type actually is assigned
> to itself or copied, which maybe they aren't in the current code. The
> missing dtor means the array is always leaked.

And this seems wasteful:

// Update the list of compiler-maintained enabled exceptions.
extern "C"
void
__gg__stash_exceptions( size_t nec, cbl_enabled_exception_t *ecs )
{
  enabled_ECs = cbl_enabled_exceptions_array_t(nec, ecs);

  if( false && getenv("match_declarative") )
    warnx("%s: %zu exceptions enabled", __func__, nec);
}

A temporary cbl_enabled_exceptions_array_t is created, which allocates a new
array of cbl_enabled_exception_t objects, then that is assigned to enabled_ECs
which allocates *another* array of those objects to copy from the temporary.
Then we leak the memory allocated by the temporary.

Looks like it would benefit from a move assignment operator so that the
assignment doesn't need to allocate twice.

Or even better, just add a reset(size_t nec, cbl_enabled_exception_t *ecs)
member to the cbl_enabled_exceptions_array_t class, and then that function
becomes:

extern "C"
void
__gg__stash_exceptions( size_t nec, cbl_enabled_exception_t *ecs )
{
  enabled_ECs.reset(nec, ecs);
}

If enabled_ECs.nec >= nec then there's no need for any allocation at all, no
temporary that needs to allocate and then be copied.

Reply via email to