Skip to content

Commit 07e5952

Browse files
chdinhchdinh
committed
refactor: replace raw new/delete with std::unique_ptr in factory functions
Convert factory functions in mne library to return std::unique_ptr: - MNESssData::read(), read_from_node() - MNEProjOp::dup(), read(), read_from_node(), create_average_eeg_ref() - MNESurface::read_bem_surface() (all overloads), read_bem_surface2() Replace raw new/delete with RAII patterns: - mne_cov_matrix.cpp: FiffSparseMatrix ownership via unique_ptr - mne_source_space.cpp: FilterThreadArg ownership via unique_ptr vector - inv_guess_data.cpp: fix memory leaks in inner_skull allocation Update all callers in fwd_bem_model.cpp, mne_meas_data.cpp, mne_raw_data.cpp, and 6 test files. Net: -24 lines, eliminates ~40 raw new/delete pairs.
1 parent a367c7c commit 07e5952

File tree

18 files changed

+134
-158
lines changed

18 files changed

+134
-158
lines changed

‎src/libraries/fwd/fwd_bem_model.cpp‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -302,21 +302,19 @@ FwdBemModel::UPtr FwdBemModel::fwd_bem_load_surfaces(const QString &name, const
302302

303303
for (k = 0; k < nkind; k++) {
304304
float cond = -1.0f;
305-
MNESurface* s = MNESurface::read_bem_surface(name,kinds[k],true,cond);
306-
if (s == nullptr)
305+
auto s = MNESurface::read_bem_surface(name,kinds[k],true,cond);
306+
if (!s)
307307
return nullptr;
308308
if (cond < 0.0) {
309309
qCritical("No conductivity available for surface %s",fwd_bem_explain_surface(kinds[k]).toUtf8().constData());
310-
delete s;
311310
return nullptr;
312311
}
313312
if (s->coord_frame != FIFFV_COORD_MRI) {
314313
qCritical("FsSurface %s not specified in MRI coordinates.",fwd_bem_explain_surface(kinds[k]).toUtf8().constData());
315-
delete s;
316314
return nullptr;
317315
}
318316
sigma_tmp[k] = cond;
319-
surfs.push_back(std::shared_ptr<MNESurface>(s));
317+
surfs.push_back(std::move(s));
320318
}
321319
auto m = std::make_unique<FwdBemModel>();
322320

@@ -509,18 +507,17 @@ MNESurface::UPtr FwdBemModel::make_guesses(MNESurface* guess_surf, float guessra
509507

510508
bemname = bemFile.fileName();
511509

512-
MNESurface* sphere = MNESurface::read_bem_surface(bemname,9003,false);
513-
if (!sphere)
510+
sphere_owner = MNESurface::read_bem_surface(bemname,9003,false);
511+
if (!sphere_owner)
514512
return res;
515-
sphere_owner.reset(sphere);
516513

517-
for (k = 0; k < sphere->np; k++) {
518-
dist = sphere->point(k).norm();
519-
sphere->rr.row(k) = (guessrad * sphere->rr.row(k) / dist) + guess_r0.transpose();
514+
for (k = 0; k < sphere_owner->np; k++) {
515+
dist = sphere_owner->point(k).norm();
516+
sphere_owner->rr.row(k) = (guessrad * sphere_owner->rr.row(k) / dist) + guess_r0.transpose();
520517
}
521-
if (sphere->add_geometry_info(true) == FAIL)
518+
if (sphere_owner->add_geometry_info(true) == FAIL)
522519
return res;
523-
guess_surf = sphere;
520+
guess_surf = sphere_owner.get();
524521
}
525522
else {
526523
qInfo("Guess surface (%d = %s) is in %s coordinates",

‎src/libraries/inv/dipole_fit/inv_guess_data.cpp‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ InvGuessData::InvGuessData(const QString &guessname, const QString &guess_surfna
140140
}
141141
else {
142142
MNESurface* inner_skull = nullptr;
143-
int free_inner_skull = false;
143+
std::unique_ptr<MNESurface> inner_skull_owned;
144144
Eigen::Vector3f r0 = f->r0;
145145

146146
Q_ASSERT(f->mri_head_t);
@@ -152,15 +152,14 @@ InvGuessData::InvGuessData(const QString &guessname, const QString &guess_surfna
152152
}
153153
else if (!guess_surfname.isEmpty()) {
154154
qInfo("Reading inner skull surface from %s...\n",guess_surfname.toUtf8().data());
155-
if ((inner_skull = MNESurface::read_bem_surface(guess_surfname,FIFFV_BEM_SURF_ID_BRAIN,true)) == nullptr)
155+
inner_skull_owned = MNESurface::read_bem_surface(guess_surfname,FIFFV_BEM_SURF_ID_BRAIN,true);
156+
if (!inner_skull_owned)
156157
return;
157-
free_inner_skull = true;
158+
inner_skull = inner_skull_owned.get();
158159
}
159-
guesses.reset((MNESourceSpace*)FwdBemModel::make_guesses(inner_skull,guessrad,r0,grid,exclude,mindist).release());
160+
guesses.reset(reinterpret_cast<MNESourceSpace*>(FwdBemModel::make_guesses(inner_skull,guessrad,r0,grid,exclude,mindist).release()));
160161
if (!guesses)
161162
return;
162-
if (free_inner_skull)
163-
delete inner_skull;
164163
}
165164
{
166165
std::vector<std::unique_ptr<MNESourceSpace>> guesses_vec;
@@ -232,7 +231,7 @@ InvGuessData::InvGuessData(const QString &guessname, const QString &guess_surfna
232231
}
233232
else {
234233
MNESurface* inner_skull = nullptr;
235-
int free_inner_skull = false;
234+
std::unique_ptr<MNESurface> inner_skull_owned;
236235
Eigen::Vector3f r0 = f->r0;
237236

238237
Q_ASSERT(f->mri_head_t);
@@ -244,15 +243,14 @@ InvGuessData::InvGuessData(const QString &guessname, const QString &guess_surfna
244243
}
245244
else if (!guess_surfname.isEmpty()) {
246245
qInfo("Reading inner skull surface from %s...\n",guess_surfname.toUtf8().data());
247-
if ((inner_skull = MNESurface::read_bem_surface(guess_surfname,FIFFV_BEM_SURF_ID_BRAIN,true)) == nullptr)
246+
inner_skull_owned = MNESurface::read_bem_surface(guess_surfname,FIFFV_BEM_SURF_ID_BRAIN,true);
247+
if (!inner_skull_owned)
248248
return;
249-
free_inner_skull = true;
249+
inner_skull = inner_skull_owned.get();
250250
}
251-
guesses.reset((MNESourceSpace*)FwdBemModel::make_guesses(inner_skull,guessrad,r0,grid,exclude,mindist).release());
251+
guesses.reset(reinterpret_cast<MNESourceSpace*>(FwdBemModel::make_guesses(inner_skull,guessrad,r0,grid,exclude,mindist).release()));
252252
if (!guesses)
253253
return;
254-
if (free_inner_skull)
255-
delete inner_skull;
256254
}
257255
/*
258256
* Save the guesses for future use

‎src/libraries/mne/mne_cov_matrix.cpp‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ std::unique_ptr<MNECovMatrix> MNECovMatrix::read(const QString& name, int kind)
159159
int nnames = 0;
160160
Eigen::VectorXd cov;
161161
Eigen::VectorXd cov_diag;
162-
FiffSparseMatrix* cov_sparse = nullptr;
162+
std::unique_ptr<FiffSparseMatrix> cov_sparse_owner;
163163
Eigen::VectorXd lambda;
164164
Eigen::Matrix<float, Eigen::Dynamic, Eigen::Dynamic, Eigen::RowMajor> eigen;
165165
MatrixXf tmp_eigen;
@@ -270,20 +270,18 @@ std::unique_ptr<MNECovMatrix> MNECovMatrix::read(const QString& name, int kind)
270270
for (p = 0; p < nn; p++)
271271
cov[p] = f[p];
272272
} else {
273-
auto cov_sparse_uptr = FiffSparseMatrix::fiff_get_float_sparse_matrix(t_pTag);
274-
if (!cov_sparse_uptr) {
273+
cov_sparse_owner = FiffSparseMatrix::fiff_get_float_sparse_matrix(t_pTag);
274+
if (!cov_sparse_owner) {
275275
stream->close();
276276
return nullptr;
277277
}
278-
cov_sparse = cov_sparse_uptr.release();
279278
}
280279

281280
if (nodes[k]->find_tag(stream, FIFF_MNE_COV_EIGENVALUES, t_pTag)) {
282281
const double *lambda_data = static_cast<const double *>(t_pTag->toDouble());
283282
lambda = Eigen::Map<const Eigen::VectorXd>(lambda_data, ncov);
284283
if (nodes[k]->find_tag(stream, FIFF_MNE_COV_EIGENVECTORS, t_pTag)) {
285284
stream->close();
286-
delete cov_sparse;
287285
return nullptr;
288286
}
289287

@@ -296,19 +294,17 @@ std::unique_ptr<MNECovMatrix> MNECovMatrix::read(const QString& name, int kind)
296294
/*
297295
* Read the optional projection operator
298296
*/
299-
op.reset(MNEProjOp::read_from_node(stream, nodes[k]));
297+
op = MNEProjOp::read_from_node(stream, nodes[k]);
300298
if (!op) {
301299
stream->close();
302-
delete cov_sparse;
303300
return nullptr;
304301
}
305302
/*
306303
* Read the optional SSS data
307304
*/
308-
sss.reset(MNESssData::read_from_node(stream, nodes[k]));
305+
sss = MNESssData::read_from_node(stream, nodes[k]);
309306
if (!sss) {
310307
stream->close();
311-
delete cov_sparse;
312308
return nullptr;
313309
}
314310
/*
@@ -317,8 +313,8 @@ std::unique_ptr<MNECovMatrix> MNECovMatrix::read(const QString& name, int kind)
317313
bads = stream->read_bad_channels(nodes[k]);
318314
nbad = bads.size();
319315
}
320-
if (cov_sparse)
321-
res = create_sparse(kind, ncov, names, cov_sparse);
316+
if (cov_sparse_owner)
317+
res = create_sparse(kind, ncov, names, cov_sparse_owner.release());
322318
else if (cov.size() > 0)
323319
res = create_dense(kind, ncov, names, cov);
324320
else if (cov_diag.size() > 0)
@@ -328,7 +324,6 @@ std::unique_ptr<MNECovMatrix> MNECovMatrix::read(const QString& name, int kind)
328324
stream->close();
329325
return nullptr;
330326
}
331-
cov_sparse = nullptr;
332327
res->eigen = std::move(eigen);
333328
res->lambda = std::move(lambda);
334329
res->nfree = nfree;
@@ -370,7 +365,7 @@ std::unique_ptr<MNECovMatrix> MNECovMatrix::dup() const
370365
}
371366
res->bads = bads;
372367
res->nbad = nbad;
373-
res->proj.reset(proj ? proj->dup() : nullptr);
368+
res->proj = proj ? proj->dup() : nullptr;
374369
if (sss)
375370
res->sss = std::make_unique<MNESssData>(*sss);
376371

@@ -818,8 +813,8 @@ std::unique_ptr<MNECovMatrix> MNECovMatrix::pick_chs_omit(const QStringList& new
818813

819814
res->bads = bads;
820815
res->nbad = nbad;
821-
res->proj.reset(proj ? proj->dup() : nullptr);
822-
res->sss.reset(sss ? new MNESssData(*sss) : nullptr);
816+
res->proj = proj ? proj->dup() : nullptr;
817+
res->sss = sss ? std::make_unique<MNESssData>(*sss) : nullptr;
823818

824819
if (ch_class.size() > 0) {
825820
res->ch_class.resize(res->ncov);

‎src/libraries/mne/mne_meas_data.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -328,15 +328,15 @@ MNEMeasData *MNEMeasData::mne_read_meas_data_add(const QString &name,
328328
new_data->op = op; /* Attach inverse operator */
329329
new_data->fwd = fwd; /* ...or a fwd operator */
330330
if (op) { /* Attach the projection operator and CTF compensation info to the data, too */
331-
new_data->proj.reset(MNEProjOp::read(name));
331+
new_data->proj = MNEProjOp::read(name);
332332
if (new_data->proj && new_data->proj->nitems > 0) {
333333
qInfo("\tLoaded projection from %s:\n",name.toUtf8().data());
334334
QTextStream errStream(stderr);
335335
new_data->proj->report(errStream, QStringLiteral("\t\t"));
336336
}
337337
}
338338
else {
339-
new_data->proj.reset(MNEProjOp::read(name));
339+
new_data->proj = MNEProjOp::read(name);
340340
if (new_data->proj && new_data->proj->nitems > 0) {
341341
qInfo("\tLoaded projection from %s:\n",name.toUtf8().data());
342342
QTextStream errStream(stderr);

‎src/libraries/mne/mne_proj_op.cpp‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -169,12 +169,12 @@ void MNEProjOp::add_item(const MNENamedMatrix *vecs, int kind, const QString& de
169169

170170
//=============================================================================================================
171171

172-
MNEProjOp *MNEProjOp::dup() const
172+
std::unique_ptr<MNEProjOp> MNEProjOp::dup() const
173173
/*
174174
* Provide a duplicate (item data only)
175175
*/
176176
{
177-
MNEProjOp* res = new MNEProjOp();
177+
auto res = std::make_unique<MNEProjOp>();
178178

179179
for (int k = 0; k < nitems; k++) {
180180
const auto& it = items[k];
@@ -186,15 +186,14 @@ MNEProjOp *MNEProjOp::dup() const
186186

187187
//=============================================================================================================
188188

189-
MNEProjOp *MNEProjOp::create_average_eeg_ref(const QList<FiffChInfo>& chs, int nch)
189+
std::unique_ptr<MNEProjOp> MNEProjOp::create_average_eeg_ref(const QList<FiffChInfo>& chs, int nch)
190190
/*
191191
* Make the projection operator for average electrode reference
192192
*/
193193
{
194194
int eegcount = 0;
195195
int k;
196196
QStringList names;
197-
MNEProjOp* op;
198197

199198
for (k = 0; k < nch; k++)
200199
if (chs.at(k).kind == FIFFV_EEG_CH)
@@ -213,7 +212,7 @@ MNEProjOp *MNEProjOp::create_average_eeg_ref(const QList<FiffChInfo>& chs, int n
213212
QStringList emptyList;
214213
auto vecs = MNENamedMatrix::build(1,eegcount,emptyList,names,vec_data);
215214

216-
op = new MNEProjOp();
215+
auto op = std::make_unique<MNEProjOp>();
217216
op->add_item(vecs.get(),FIFFV_MNE_PROJ_ITEM_EEG_AVREF,"Average EEG reference");
218217

219218
return op;
@@ -280,12 +279,11 @@ int MNEProjOp::project_vector(Eigen::Ref<Eigen::VectorXf> vec, bool do_complemen
280279

281280
//=============================================================================================================
282281

283-
MNEProjOp *MNEProjOp::read_from_node(FiffStream::SPtr &stream, const FiffDirNode::SPtr &start)
282+
std::unique_ptr<MNEProjOp> MNEProjOp::read_from_node(FiffStream::SPtr &stream, const FiffDirNode::SPtr &start)
284283
/*
285284
* Load all the linear projection data
286285
*/
287286
{
288-
MNEProjOp* op = nullptr;
289287
QList<FiffDirNode::SPtr> proj;
290288
FiffDirNode::SPtr start_node;
291289
QList<FiffDirNode::SPtr> items;
@@ -309,7 +307,7 @@ MNEProjOp *MNEProjOp::read_from_node(FiffStream::SPtr &stream, const FiffDirNode
309307
else
310308
start_node = start;
311309

312-
op = new MNEProjOp();
310+
auto op = std::make_unique<MNEProjOp>();
313311
proj = start_node->dir_tree_find(FIFFB_PROJ);
314312
if (proj.size() == 0 || proj[0]->isEmpty()) /* The caller must recognize an empty projection */
315313
return op;
@@ -366,41 +364,41 @@ MNEProjOp *MNEProjOp::read_from_node(FiffStream::SPtr &stream, const FiffDirNode
366364
}
367365
if (item_nchan <= 0) {
368366
qCritical("Number of channels incorrectly specified for one of the projection items.");
369-
delete op; return nullptr;
367+
return nullptr;
370368
}
371369
/*
372370
* Take care of the channel names
373371
*/
374372
if (!node->find_tag(stream, FIFF_PROJ_ITEM_CH_NAME_LIST, t_pTag)) {
375-
delete op; return nullptr;
373+
return nullptr;
376374
}
377375

378376
item_names = FiffStream::split_name_list(t_pTag->toString());
379377

380378
if (item_names.size() != item_nchan) {
381379
qCritical("Channel name list incorrectly specified for proj item # %d",k+1);
382380
item_names.clear();
383-
delete op; return nullptr;
381+
return nullptr;
384382
}
385383
/*
386384
* Kind of item
387385
*/
388386
if (!node->find_tag(stream, FIFF_PROJ_ITEM_KIND, t_pTag)) {
389-
delete op; return nullptr;
387+
return nullptr;
390388
}
391389
item_kind = *t_pTag->toInt();
392390
/*
393391
* How many vectors
394392
*/
395393
if (!node->find_tag(stream,FIFF_PROJ_ITEM_NVEC, t_pTag)) {
396-
delete op; return nullptr;
394+
return nullptr;
397395
}
398396
item_nvec = *t_pTag->toInt();
399397
/*
400398
* The projection data
401399
*/
402400
if (!node->find_tag(stream,FIFF_PROJ_ITEM_VECTORS, t_pTag)) {
403-
delete op; return nullptr;
401+
return nullptr;
404402
}
405403

406404
MatrixXf item_vectors = t_pTag->toFloatMatrix().transpose();
@@ -427,18 +425,16 @@ MNEProjOp *MNEProjOp::read_from_node(FiffStream::SPtr &stream, const FiffDirNode
427425

428426
//=============================================================================================================
429427

430-
MNEProjOp *MNEProjOp::read(const QString &name)
428+
std::unique_ptr<MNEProjOp> MNEProjOp::read(const QString &name)
431429
{
432430
QFile file(name);
433431
FiffStream::SPtr stream(new FiffStream(&file));
434432

435433
if(!stream->open())
436434
return nullptr;
437435

438-
MNEProjOp* res = nullptr;
439-
440436
FiffDirNode::SPtr t_default;
441-
res = read_from_node(stream,t_default);
437+
auto res = read_from_node(stream,t_default);
442438

443439
stream->close();
444440

‎src/libraries/mne/mne_proj_op.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ class MNESHARED_EXPORT MNEProjOp
153153
*
154154
* @return A newly allocated copy. Caller takes ownership.
155155
*/
156-
MNEProjOp* dup() const;
156+
std::unique_ptr<MNEProjOp> dup() const;
157157

158158
/**
159159
* @brief Create an average EEG reference projector.
@@ -167,7 +167,7 @@ class MNESHARED_EXPORT MNEProjOp
167167
* @return A new projection operator, or NULL if no EEG channels are found.
168168
* Caller takes ownership.
169169
*/
170-
static MNEProjOp* create_average_eeg_ref(const QList<FIFFLIB::FiffChInfo>& chs, int nch);
170+
static std::unique_ptr<MNEProjOp> create_average_eeg_ref(const QList<FIFFLIB::FiffChInfo>& chs, int nch);
171171

172172
/**
173173
* Count how many active projection vectors affect a given list of
@@ -216,7 +216,7 @@ class MNESHARED_EXPORT MNEProjOp
216216
* @return A populated projection operator (possibly with zero items),
217217
* or NULL on error. Caller takes ownership.
218218
*/
219-
static MNEProjOp* read_from_node(FIFFLIB::FiffStream::SPtr& stream,
219+
static std::unique_ptr<MNEProjOp> read_from_node(FIFFLIB::FiffStream::SPtr& stream,
220220
const FIFFLIB::FiffDirNode::SPtr& start);
221221

222222
/**
@@ -228,7 +228,7 @@ class MNESHARED_EXPORT MNEProjOp
228228
* @return The loaded projection operator, or NULL on error.
229229
* Caller takes ownership.
230230
*/
231-
static MNEProjOp* read(const QString& name);
231+
static std::unique_ptr<MNEProjOp> read(const QString& name);
232232

233233
/**
234234
* @brief Load and combine SSP projection operators from files for the selected channels.

‎src/libraries/mne/mne_raw_data.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1374,7 +1374,7 @@ MNERawData *MNERawData::open_file_comp(const QString& name,
13741374
/*
13751375
* SSS data
13761376
*/
1377-
data->sss.reset(MNESssData::read(data->filename));
1377+
data->sss = MNESssData::read(data->filename);
13781378
if (data->sss && data->sss->job != FIFFV_SSS_JOB_NOTHING && data->sss->comp_info.size() > 0) {
13791379
qInfo("SSS data read from %s :\n",data->filename.toUtf8().constData());
13801380
QTextStream errStream(stderr);

0 commit comments

Comments
 (0)