-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
netCDF: Add BAND_NAMES creation option #9657
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code LGTM, the codecheck CI failure seems like a false positive. As a workaround you can possibly check for poDS != nullptr.
The other gcore test failure seems unrelated.
frmts/netcdf/netcdfdataset.cpp
Outdated
@@ -9104,7 +9104,7 @@ netCDFDataset *netCDFDataset::CreateLL(const char *pszFilename, int nXSize, | |||
|
|||
CPLReleaseMutex(hNCMutex); // Release mutex otherwise we'll deadlock with | |||
// GDALDataset own mutex. | |||
netCDFDataset *poDS = new netCDFDataset(); | |||
auto poDS = std::make_unique<netCDFDataset>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not totally sure this change is safe. See the comments around the explicit "delete poDS" about mutex deadlocks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Release mutex otherwise we'll deadlock with
// GDALDataset own mutex.
Is this referring to use of hNCMutex
by netCDFDataset
? Or is it referring to the m_poPrivate->hMutex
in GDALDataset
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this was a deadlock between hNCMutex and
Line 324 in 732d82c
CPLMutexHolderD(&hDLMutex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed this change.
2c43578
to
dcfdfa9
Compare
What does this PR do?
Adds an option to control the names of netCDF variables created by GDAL.
What are related issues/pull requests?
#1427
https://gis.stackexchange.com/questions/230642/setting-band-names-when-writing-multiple-layer-rasters-using-gdal-with-python
https://gis.stackexchange.com/questions/265204/gdal-creation-option-to-set-variable-name-to-netcdf
Tasklist