Skip to content

Commit fce174e

Browse files
authored
Replace NXhandle* with NXhandle& (#39425)
### Description of work #### Summary of work `napi` was apparently written before pass-by-reference was a thing. Or at least it did not make proper use of pass-by-reference, instead passing `void**` objects to enable edited the `void*` in certain methods. This replaces the four methods in `napi` and `napi5` that use `void**` to instead use `void*&`. Refs #38332 #### Further detail of work Using pass-by-reference means the `void*` object can be directly modified, without having to use dereferencing. This impacted the internals of the methods. The few places in the code that call these methods were also changed, including in the `napi` tests. ### To test: This is a minor refactor that should not change behavior. If NeXus files can still be opened, closed, reopened, and flushed (which the existing unit tests prove) then everything is fine. *This does not require release notes* because it is way under the hood, away from users. --- ### Reviewer Please comment on the points listed below ([full description](http://developer.mantidproject.org/ReviewingAPullRequest.html)). **Your comments will be used as part of the gatekeeper process, so please comment clearly on what you have checked during your review.** If changes are made to the PR during the review process then your final comment will be the most important for gatekeepers. In this comment you should make it clear why any earlier review is still valid, or confirm that all requested changes have been addressed. #### Code Review - Is the code of an acceptable quality? - Does the code conform to the [coding standards](http://developer.mantidproject.org/Standards/)? - Are the unit tests small and test the class in isolation? - If there is GUI work does it follow the [GUI standards](http://developer.mantidproject.org/Standards/GUIStandards.html)? - If there are changes in the release notes then do they describe the changes appropriately? - Do the release notes conform to the [release notes guide](https://developer.mantidproject.org/Standards/ReleaseNotesGuide.html)? #### Functional Tests - Do changes function as described? Add comments below that describe the tests performed? - Do the changes handle unexpected situations, e.g. bad input? - Has the relevant (user and developer) documentation been added/updated? Does everything look good? Mark the review as **Approve**. A member of `@mantidproject/gatekeepers` will take care of it. ### Gatekeeper If you need to request changes to a PR then please add a comment and set the review status to "Request changes". This will stop the PR from showing up in the list for other gatekeepers.
1 parent d073744 commit fce174e

File tree

11 files changed

+64
-64
lines changed

11 files changed

+64
-64
lines changed

Framework/Nexus/inc/MantidNexus/napi.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,14 +150,14 @@ extern "C" {
150150
* \return NX_OK on success, NX_ERROR in the case of an error.
151151
* \ingroup c_init
152152
*/
153-
MANTID_NEXUS_DLL NXstatus NXopen(CONSTCHAR *filename, NXaccess access_method, NXhandle *pHandle);
153+
MANTID_NEXUS_DLL NXstatus NXopen(CONSTCHAR *filename, NXaccess access_method, NXhandle &handle);
154154

155155
/**
156156
* Opens an existing NeXus file a second time for e.g. access from another thread.
157157
* \return NX_OK on success, NX_ERROR in the case of an error.
158158
* \ingroup c_init
159159
*/
160-
MANTID_NEXUS_DLL NXstatus NXreopen(NXhandle pOrigHandle, NXhandle *pNewHandle);
160+
MANTID_NEXUS_DLL NXstatus NXreopen(NXhandle pOrigHandle, NXhandle &pNewHandle);
161161

162162
/**
163163
* close a NeXus file
@@ -166,15 +166,15 @@ MANTID_NEXUS_DLL NXstatus NXreopen(NXhandle pOrigHandle, NXhandle *pNewHandle);
166166
* \return NX_OK on success, NX_ERROR in the case of an error.
167167
* \ingroup c_init
168168
*/
169-
MANTID_NEXUS_DLL NXstatus NXclose(NXhandle *pHandle);
169+
MANTID_NEXUS_DLL NXstatus NXclose(NXhandle &handle);
170170

171171
/**
172172
* flush data to disk
173173
* \param pHandle A NeXus file handle as initialized by NXopen.
174174
* \return NX_OK on success, NX_ERROR in the case of an error.
175175
* \ingroup c_readwrite
176176
*/
177-
MANTID_NEXUS_DLL NXstatus NXflush(NXhandle *pHandle);
177+
MANTID_NEXUS_DLL NXstatus NXflush(NXhandle &handle);
178178

179179
/**
180180
* NeXus groups are NeXus way of structuring information into a hierarchy.

Framework/Nexus/inc/MantidNexus/napi5.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@
99

1010
/* HDF5 interface */
1111

12-
extern NXstatus NX5open(CONSTCHAR *filename, NXaccess access_method, NXhandle *pHandle);
13-
extern NXstatus NX5reopen(NXhandle pOrigHandle, NXhandle *pNewHandle);
12+
extern NXstatus NX5open(CONSTCHAR *filename, NXaccess access_method, NXhandle &handle);
13+
extern NXstatus NX5reopen(NXhandle origHandle, NXhandle &newHandle);
1414

15-
extern NXstatus NX5close(NXhandle *pHandle);
16-
extern NXstatus NX5flush(NXhandle *pHandle);
15+
extern NXstatus NX5close(NXhandle &handle);
16+
extern NXstatus NX5flush(NXhandle &handle);
1717

1818
extern NXstatus NX5makegroup(NXhandle handle, CONSTCHAR *name, CONSTCHAR *NXclass);
1919
extern NXstatus NX5opengroup(NXhandle handle, CONSTCHAR *name, CONSTCHAR *NXclass);

Framework/Nexus/inc/MantidNexus/napi_internal.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@
3333

3434
typedef struct {
3535
NXhandle pNexusData;
36-
NXstatus (*nxreopen)(NXhandle pOrigHandle, NXhandle *pNewHandle);
37-
NXstatus (*nxclose)(NXhandle *pHandle);
38-
NXstatus (*nxflush)(NXhandle *pHandle);
36+
NXstatus (*nxreopen)(NXhandle origHandle, NXhandle &newHandle);
37+
NXstatus (*nxclose)(NXhandle &handle);
38+
NXstatus (*nxflush)(NXhandle &handle);
3939
NXstatus (*nxmakegroup)(NXhandle handle, CONSTCHAR *name, CONSTCHAR *NXclass);
4040
NXstatus (*nxopengroup)(NXhandle handle, CONSTCHAR *name, CONSTCHAR *NXclass);
4141
NXstatus (*nxclosegroup)(NXhandle handle);

Framework/Nexus/src/NexusFile.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ void File::initOpenFile(const string &filename, const NXaccess access) {
105105
}
106106

107107
NXhandle temp;
108-
NXstatus status = NXopen(filename.c_str(), access, &(temp));
108+
NXstatus status = NXopen(filename.c_str(), access, temp);
109109
if (status != NXstatus::NX_OK) {
110110
stringstream msg;
111111
msg << "NXopen(" << filename << ", " << access << ") failed";
@@ -145,7 +145,7 @@ File &File::operator=(File const &f) {
145145

146146
File::~File() {
147147
if (m_close_handle && m_pfile_id != NULL) {
148-
NXstatus status = NXclose(&(*this->m_pfile_id));
148+
NXstatus status = NXclose(*m_pfile_id);
149149
this->m_pfile_id = NULL;
150150
if (status != NXstatus::NX_OK) {
151151
stringstream msg;
@@ -157,12 +157,12 @@ File::~File() {
157157

158158
void File::close() {
159159
if (this->m_pfile_id != NULL) {
160-
NAPI_CALL(NXclose(&(*this->m_pfile_id)), "NXclose failed");
160+
NAPI_CALL(NXclose(*m_pfile_id), "NXclose failed");
161161
this->m_pfile_id = NULL;
162162
}
163163
}
164164

165-
void File::flush() { NAPI_CALL(NXflush(&(*this->m_pfile_id)), "NXflush failed"); }
165+
void File::flush() { NAPI_CALL(NXflush(*m_pfile_id), "NXflush failed"); }
166166

167167
//------------------------------------------------------------------------------------------------------------------
168168
// FILE NAVIGATION METHODS

Framework/Nexus/src/napi.cpp

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -98,19 +98,19 @@ static pNexusFunction handleToNexusFunc(NXhandle fid) {
9898
/*--------------------------------------------------------------------*/
9999
static NXstatus NXinternalopen(CONSTCHAR *userfilename, NXaccess am, pFileStack fileStack);
100100
/*----------------------------------------------------------------------*/
101-
NXstatus NXopen(CONSTCHAR *userfilename, NXaccess am, NXhandle *gHandle) {
101+
NXstatus NXopen(CONSTCHAR *userfilename, NXaccess am, NXhandle &gHandle) {
102102
NXstatus status;
103103
pFileStack fileStack = NULL;
104104

105-
*gHandle = NULL;
105+
gHandle = NULL;
106106
fileStack = makeFileStack();
107107
if (fileStack == NULL) {
108108
NXReportError("ERROR: no memory to create filestack");
109109
return NXstatus::NX_ERROR;
110110
}
111111
status = NXinternalopen(userfilename, am, fileStack);
112112
if (status == NXstatus::NX_OK) {
113-
*gHandle = fileStack;
113+
gHandle = fileStack;
114114
}
115115

116116
return status;
@@ -148,7 +148,7 @@ static NXstatus NXinternalopen(CONSTCHAR *userfilename, NXaccess am, pFileStack
148148
}
149149

150150
NXhandle hdf5_handle = NULL;
151-
NXstatus retstat = NX5open(userfilename, am, &hdf5_handle);
151+
NXstatus retstat = NX5open(userfilename, am, hdf5_handle);
152152
if (retstat != NXstatus::NX_OK) {
153153
free(fHandle);
154154
} else {
@@ -161,11 +161,11 @@ static NXstatus NXinternalopen(CONSTCHAR *userfilename, NXaccess am, pFileStack
161161
return retstat;
162162
}
163163

164-
NXstatus NXreopen(NXhandle pOrigHandle, NXhandle *pNewHandle) {
164+
NXstatus NXreopen(NXhandle pOrigHandle, NXhandle &newHandle) {
165165
pFileStack newFileStack;
166166
pFileStack origFileStack = static_cast<pFileStack>(pOrigHandle);
167167
pNexusFunction fOrigHandle = NULL, fNewHandle = NULL;
168-
*pNewHandle = NULL;
168+
newHandle = NULL;
169169
newFileStack = makeFileStack();
170170
if (newFileStack == NULL) {
171171
NXReportError("ERROR: no memory to create filestack");
@@ -184,32 +184,32 @@ NXstatus NXreopen(NXhandle pOrigHandle, NXhandle *pNewHandle) {
184184
}
185185
fNewHandle = static_cast<NexusFunction *>(malloc(sizeof(NexusFunction)));
186186
memcpy(fNewHandle, fOrigHandle, sizeof(NexusFunction));
187-
fNewHandle->nxreopen(fOrigHandle->pNexusData, &(fNewHandle->pNexusData));
187+
fNewHandle->nxreopen(fOrigHandle->pNexusData, fNewHandle->pNexusData);
188188
pushFileStack(newFileStack, fNewHandle, peekFilenameOnStack(origFileStack));
189-
*pNewHandle = newFileStack;
189+
newHandle = newFileStack;
190190
return NXstatus::NX_OK;
191191
}
192192

193193
/* ------------------------------------------------------------------------- */
194194

195-
NXstatus NXclose(NXhandle *fid) {
195+
NXstatus NXclose(NXhandle &fid) {
196196
NXhandle hfil;
197197
NXstatus status;
198198
pFileStack fileStack = NULL;
199199
pNexusFunction pFunc = NULL;
200-
if (*fid == NULL) {
200+
if (fid == NULL) {
201201
return NXstatus::NX_OK;
202202
}
203-
fileStack = (pFileStack)*fid;
203+
fileStack = static_cast<pFileStack>(fid);
204204
pFunc = peekFileOnStack(fileStack);
205205
hfil = pFunc->pNexusData;
206-
status = pFunc->nxclose(&hfil);
206+
status = pFunc->nxclose(hfil);
207207
pFunc->pNexusData = hfil;
208208
free(pFunc);
209209
popFileStack(fileStack);
210210
if (fileStackDepth(fileStack) < 0) {
211211
killFileStack(fileStack);
212-
*fid = NULL;
212+
fid = NULL;
213213
}
214214
/* we can't set fid to NULL always as the handle points to a stack of files for external file support */
215215
/*
@@ -264,7 +264,7 @@ NXstatus NXclosegroup(NXhandle fid) {
264264
NXgetgroupID(fid, &currentID);
265265
peekIDOnStack(fileStack, &closeID);
266266
if (NXsameID(fid, &closeID, &currentID) == NXstatus::NX_OK) {
267-
NXclose(&fid);
267+
NXclose(fid);
268268
status = NXclosegroup(fid);
269269
} else {
270270
status = pFunc->nxclosegroup(pFunc->pNexusData);
@@ -330,7 +330,7 @@ NXstatus NXclosedata(NXhandle fid) {
330330
NXgetdataID(fid, &currentID);
331331
peekIDOnStack(fileStack, &closeID);
332332
if (NXsameID(fid, &closeID, &currentID) == NXstatus::NX_OK) {
333-
NXclose(&fid);
333+
NXclose(fid);
334334
status = NXclosedata(fid);
335335
} else {
336336
status = pFunc->nxclosedata(pFunc->pNexusData);
@@ -420,16 +420,16 @@ NXstatus NXopensourcegroup(NXhandle fid) {
420420

421421
/*----------------------------------------------------------------------*/
422422

423-
NXstatus NXflush(NXhandle *pHandle) {
423+
NXstatus NXflush(NXhandle &handle) {
424424
NXhandle hfil;
425425
pFileStack fileStack = NULL;
426426
NXstatus status;
427427

428428
pNexusFunction pFunc = NULL;
429-
fileStack = (pFileStack)*pHandle;
429+
fileStack = static_cast<pFileStack>(handle);
430430
pFunc = peekFileOnStack(fileStack);
431431
hfil = pFunc->pNexusData;
432-
status = pFunc->nxflush(&hfil);
432+
status = pFunc->nxflush(hfil);
433433
pFunc->pNexusData = hfil;
434434
return status;
435435
}

Framework/Nexus/src/napi5.cpp

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -207,10 +207,10 @@ static void buildCurrentAddress(pNexusFile5 self, char *addressBuffer, // cppche
207207
208208
--------------------------------------------------------------------- */
209209

210-
NXstatus NX5reopen(NXhandle pOrigHandle, NXhandle *pNewHandle) {
210+
NXstatus NX5reopen(NXhandle origHandle, NXhandle &newHandle) {
211211
pNexusFile5 pNew = NULL, pOrig = NULL;
212-
*pNewHandle = NULL;
213-
pOrig = static_cast<pNexusFile5>(pOrigHandle);
212+
newHandle = NULL;
213+
pOrig = static_cast<pNexusFile5>(origHandle);
214214
pNew = static_cast<pNexusFile5>(malloc(sizeof(NexusFile5)));
215215
if (!pNew) {
216216
NXReportError("ERROR: no memory to create File datastructure");
@@ -226,7 +226,7 @@ NXstatus NX5reopen(NXhandle pOrigHandle, NXhandle *pNewHandle) {
226226
strcpy(pNew->iAccess, pOrig->iAccess);
227227
pNew->iNXID = NX5SIGNATURE;
228228
pNew->iStack5[0].iVref = 0; /* root! */
229-
*pNewHandle = static_cast<NXhandle>(pNew);
229+
newHandle = static_cast<NXhandle>(pNew);
230230
return NXstatus::NX_OK;
231231
}
232232

@@ -300,7 +300,7 @@ herr_t set_str_attribute(hid_t parent_id, CONSTCHAR *name, CONSTCHAR *buffer) {
300300
return 0;
301301
}
302302

303-
NXstatus NX5open(CONSTCHAR *filename, NXaccess am, NXhandle *pHandle) {
303+
NXstatus NX5open(CONSTCHAR *filename, NXaccess am, NXhandle &handle) {
304304
hid_t root_id;
305305
pNexusFile5 pNew = NULL;
306306
char pBuffer[512];
@@ -309,7 +309,7 @@ NXstatus NX5open(CONSTCHAR *filename, NXaccess am, NXhandle *pHandle) {
309309
unsigned int vers_major, vers_minor, vers_release, am1;
310310
hid_t fapl = -1;
311311

312-
*pHandle = NULL;
312+
handle = NULL;
313313

314314
if (H5get_libversion(&vers_major, &vers_minor, &vers_release) < 0) {
315315
NXReportError("ERROR: cannot determine HDF5 library version");
@@ -422,17 +422,17 @@ NXstatus NX5open(CONSTCHAR *filename, NXaccess am, NXhandle *pHandle) {
422422
}
423423
pNew->iNXID = NX5SIGNATURE;
424424
pNew->iStack5[0].iVref = 0; /* root! */
425-
*pHandle = static_cast<NXhandle>(pNew);
425+
handle = static_cast<NXhandle>(pNew);
426426
return NXstatus::NX_OK;
427427
}
428428

429429
/* ------------------------------------------------------------------------- */
430430

431-
NXstatus NX5close(NXhandle *fid) {
431+
NXstatus NX5close(NXhandle &fid) {
432432
pNexusFile5 pFile = NULL;
433433
herr_t iRet;
434434

435-
pFile = NXI5assert(*fid);
435+
pFile = NXI5assert(fid);
436436

437437
iRet = 0;
438438
/*
@@ -467,7 +467,7 @@ NXstatus NX5close(NXhandle *fid) {
467467
free(pFile->iCurrentLD);
468468
}
469469
free(pFile);
470-
*fid = NULL;
470+
fid = NULL;
471471
H5garbage_collect();
472472
return NXstatus::NX_OK;
473473
}
@@ -1327,11 +1327,11 @@ NXstatus NX5makelink(NXhandle fid, NXlink *sLink) {
13271327

13281328
/*----------------------------------------------------------------------*/
13291329

1330-
NXstatus NX5flush(NXhandle *pHandle) {
1330+
NXstatus NX5flush(NXhandle &handle) {
13311331
pNexusFile5 pFile = NULL;
13321332
herr_t iRet;
13331333

1334-
pFile = NXI5assert(*pHandle);
1334+
pFile = NXI5assert(handle);
13351335
if (pFile->iCurrentD != 0) {
13361336
iRet = H5Fflush(pFile->iCurrentD, H5F_SCOPE_LOCAL);
13371337
} else if (pFile->iCurrentG != 0) {

Framework/Nexus/test/leak_test1.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,20 @@ int main() {
1616
NXhandle fileid;
1717

1818
removeFile(szFile); // in case it was left over from previous run
19-
if (NXopen(szFile.c_str(), access_mode, &fileid) != NXstatus::NX_OK)
19+
if (NXopen(szFile.c_str(), access_mode, fileid) != NXstatus::NX_OK)
2020
ON_ERROR("NXopen failed!\n")
2121

22-
if (NXclose(&fileid) != NXstatus::NX_OK)
22+
if (NXclose(fileid) != NXstatus::NX_OK)
2323
ON_ERROR("NXclose failed!\n")
2424

2525
for (iReOpen = 0; iReOpen < nReOpen; iReOpen++) {
2626
if (0 == iReOpen % 100)
2727
printf("loop count %d\n", iReOpen);
2828

29-
if (NXopen(szFile.c_str(), NXACC_RDWR, &fileid) != NXstatus::NX_OK)
29+
if (NXopen(szFile.c_str(), NXACC_RDWR, fileid) != NXstatus::NX_OK)
3030
ON_ERROR("NXopen failed!\n");
3131

32-
if (NXclose(&fileid) != NXstatus::NX_OK)
32+
if (NXclose(fileid) != NXstatus::NX_OK)
3333
ON_ERROR("NXclose failed!\n");
3434
}
3535

Framework/Nexus/test/leak_test2.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ int main() {
2626
remove(strFile);
2727
printf("file %s\n", strFile);
2828
NXhandle fileid;
29-
if (NXopen(strFile, access_mode, &fileid) != NXstatus::NX_OK) {
29+
if (NXopen(strFile, access_mode, fileid) != NXstatus::NX_OK) {
3030
std::cerr << "NXopen failed!" << std::endl;
3131
return 1;
3232
}
@@ -94,7 +94,7 @@ int main() {
9494
}
9595
}
9696

97-
if (NXclose(&fileid) != NXstatus::NX_OK) {
97+
if (NXclose(fileid) != NXstatus::NX_OK) {
9898
std::cerr << "NXclose failed!" << std::endl;
9999
return 1;
100100
}

Framework/Nexus/test/leak_test3.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ int main() {
3737

3838
NXhandle fileid;
3939
NXlink aLink;
40-
if (NXopen(szFile, NXACC_CREATE5, &fileid) != NXstatus::NX_OK)
40+
if (NXopen(szFile, NXACC_CREATE5, fileid) != NXstatus::NX_OK)
4141
ON_ERROR("NXopen_failed")
4242

4343
for (iEntry = 0; iEntry < nEntry; iEntry++) {
@@ -87,7 +87,7 @@ int main() {
8787
ON_ERROR("NXclosegroup failed!")
8888
}
8989

90-
if (NXclose(&fileid) != NXstatus::NX_OK)
90+
if (NXclose(fileid) != NXstatus::NX_OK)
9191
ON_ERROR("NXclose failed!")
9292

9393
// Delete file

0 commit comments

Comments
 (0)