[Bug cobol/119323] cppcheck meets libgcobol
redi at gcc dot gnu.org
gcc-bugzilla@gcc.gnu.org
Tue Mar 18 22:09:02 GMT 2025
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.
More information about the Gcc-bugs
mailing list