Context Navigation

← Previous Ticket
Next Ticket →

Ticket #9949 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

Use shared pointer in LoadEventNexus::runLoadNexusLogs

Reported by:	Wenduo Zhou	Owned by:	Wenduo Zhou
Priority:	critical	Milestone:	Release 3.3
Component:	Framework	Keywords:
Cc:	petersonpf@…	Blocked By:
Blocking:		Tester:	Martyn Gigg

Description

Method runLoadNexusLogs of algorithm LoadEventNexus uses a pointer, which is reported as a resource leak by coverity. Make it a shared pointer to fix the issue.

Change History

comment:1 Changed 6 years ago by Wenduo Zhou

Refs #9949. Replace pointer by reference.

Changeset: 3a20748b7ba3462542a6ccd52c894b6c360a451f

comment:2 Changed 6 years ago by Wenduo Zhou

Status changed from new to assigned

comment:3 Changed 6 years ago by Wenduo Zhou

Status changed from assigned to verify
Resolution set to fixed

comment:4 Changed 6 years ago by Roman Tolchenov

Status changed from verify to verifying
Tester set to Roman Tolchenov

comment:5 Changed 6 years ago by Roman Tolchenov

Status changed from verifying to reopened
Resolution fixed deleted

It's not the issue coverity reported. LoadEventNexus::runLoadNexusLogs(...) allocates an object of type BankPulseTimes and returns it. The code in LoadTOFRawNexus doesn't assign the returned address to any variable and of course doesn't free the memory.

comment:6 Changed 6 years ago by Wenduo Zhou

Status changed from reopened to inprogress

Fixed the issue described by Roman. Refs #9949.

Changeset: 0960cef370f3b68279bc7a8c27e06254c8660d8e

comment:7 Changed 6 years ago by Wenduo Zhou

Status changed from inprogress to verify
Resolution set to fixed

comment:8 Changed 6 years ago by Martyn Gigg

Status changed from verify to verifying
Tester changed from Roman Tolchenov to Martyn Gigg

comment:9 Changed 6 years ago by Martyn Gigg

Status changed from verifying to reopened
Resolution fixed deleted

I think we can fix this in a better way.

The ticket title states that runLoadNexusLogs should return a shared_ptr. I agree with that, it would avoid having to worry about the deletion of the pulse times.

My second comment is that LoadTOFRawNexus doesn't actually use the pulse times at all so it seems like a waste allocating them in the first place. What about adding an additional flag to runLoadNexusLogs, something like loadPulseTimes, which defaults to true so the behaviour is like it was before?