diff --git a/ChangeLog b/ChangeLog index 83cf770..7e6205a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -7,11 +7,12 @@ Add GCTABackground::rate_ebin() method (#2308) -2018-02-13 +2018-02-15 * Bug fix version 1.5.1 released ================================ + Prevent unintentional deallocation of arguments (#2340). Correct sky map smoothing. diff --git a/doc/source/admin/release_history/1.5.1.rst b/doc/source/admin/release_history/1.5.1.rst index fb83d44..dcfcd20 100644 --- a/doc/source/admin/release_history/1.5.1.rst +++ b/doc/source/admin/release_history/1.5.1.rst @@ -10,6 +10,8 @@ GammaLib 1.5.1 is a bug fix release for GammaLib release 1.5. The following bugs were fixed: +* [`2340 `_] - + Prevent unintentional deallocation of arguments * Correct sky map smoothing diff --git a/inst/cta/src/GCTAObservation.cpp b/inst/cta/src/GCTAObservation.cpp index f2f8de6..1f5f3d3 100644 --- a/inst/cta/src/GCTAObservation.cpp +++ b/inst/cta/src/GCTAObservation.cpp @@ -335,8 +335,11 @@ void GCTAObservation::response(const GResponse& rsp) throw GException::invalid_argument(G_RESPONSE_SET, msg); } - // Free response - if (m_response != NULL) delete m_response; + // Free existing response only if it differs from current response. This + // prevents unintential deallocation of the argument + if ((m_response != NULL) && (m_response != &rsp)) { + delete m_response; + } // Clone response function m_response = ptr->clone(); diff --git a/inst/cta/src/GCTAOnOffObservation.cpp b/inst/cta/src/GCTAOnOffObservation.cpp index 7972f88..5804411 100644 --- a/inst/cta/src/GCTAOnOffObservation.cpp +++ b/inst/cta/src/GCTAOnOffObservation.cpp @@ -575,8 +575,11 @@ void GCTAOnOffObservation::response(const GResponse& rsp) throw GException::invalid_argument(G_RESPONSE_SET, msg); } - // Free response - if (m_response != NULL) delete m_response; + // Free existing response only if it differs from current response. This + // prevents unintential deallocation of the argument + if ((m_response != NULL) && (m_response != &rsp)) { + delete m_response; + } // Clone response function m_response = ptr->clone(); diff --git a/src/fits/GFits.cpp b/src/fits/GFits.cpp index 1950e26..5bb0e78 100644 --- a/src/fits/GFits.cpp +++ b/src/fits/GFits.cpp @@ -1,7 +1,7 @@ /*************************************************************************** * GFits.cpp - FITS file access class * * ----------------------------------------------------------------------- * - * copyright (C) 2008-2017 by Juergen Knoedlseder * + * copyright (C) 2008-2018 by Juergen Knoedlseder * * ----------------------------------------------------------------------- * * * * This program is free software: you can redistribute it and/or modify * @@ -601,8 +601,11 @@ GFitsHDU* GFits::set(const int& extno, const GFitsHDU& hdu) throw GException::invalid_argument(G_SET1, msg); } - // Delete any existing HDU - if (m_hdu[extno] != NULL) delete m_hdu[extno]; + // Free existing HDU only if it differs from current HDU. This prevents + // unintential deallocation of the argument + if ((m_hdu[extno] != NULL) && (m_hdu[extno] != &hdu)) { + delete m_hdu[extno]; + } // Assign new HDU by cloning m_hdu[extno] = hdu.clone(); diff --git a/src/fits/GFitsTable.cpp b/src/fits/GFitsTable.cpp index 90c9654..1d4dff2 100644 --- a/src/fits/GFitsTable.cpp +++ b/src/fits/GFitsTable.cpp @@ -1,7 +1,7 @@ /*************************************************************************** * GFitsTable.cpp - FITS table base class * * ----------------------------------------------------------------------- * - * copyright (C) 2008-2017 by Juergen Knoedlseder * + * copyright (C) 2008-2018 by Juergen Knoedlseder * * ----------------------------------------------------------------------- * * * * This program is free software: you can redistribute it and/or modify * @@ -401,12 +401,6 @@ GFitsTableCol* GFitsTable::set(const std::string& colname, ***************************************************************************/ GFitsTableCol* GFitsTable::insert(int colnum, const GFitsTableCol& column) { - // Make sure that 'colnum' is valid. Since we add one column the total - // number of columns at return will be m_cols+1, hence the column - // index is comprised between [0,m_cols] - //if (colnum < 0) colnum = 0; - //if (colnum > m_cols) colnum = m_cols; - // Check if column number is valid if (colnum < 0 || colnum > ncols()) { throw GException::out_of_range(G_INSERT1, "Column number", colnum, ncols()+1); diff --git a/src/model/GModelSky.cpp b/src/model/GModelSky.cpp index f1019e4..8b8776a 100644 --- a/src/model/GModelSky.cpp +++ b/src/model/GModelSky.cpp @@ -1,7 +1,7 @@ /*************************************************************************** * GModelSky.cpp - Sky model class * * ----------------------------------------------------------------------- * - * copyright (C) 2011-2017 by Juergen Knoedlseder * + * copyright (C) 2011-2018 by Juergen Knoedlseder * * ----------------------------------------------------------------------- * * * * This program is free software: you can redistribute it and/or modify * @@ -411,8 +411,11 @@ GModelSky* GModelSky::clone(void) const ***************************************************************************/ void GModelSky::spatial(const GModelSpatial* spatial) { - // Free spatial model component - if (m_spatial != NULL) delete m_spatial; + // Free spatial model component only if it differs from current + // component. This prevents unintential deallocation of the argument + if ((m_spatial != NULL) && (m_spatial != spatial)) { + delete m_spatial; + } // Clone spatial model component if it exists, otherwise set pointer // to NULL @@ -438,8 +441,11 @@ void GModelSky::spatial(const GModelSpatial* spatial) ***************************************************************************/ void GModelSky::spectral(const GModelSpectral* spectral) { - // Free spectral model component - if (m_spectral != NULL) delete m_spectral; + // Free spectral model component only if it differs from current + // component. This prevents unintential deallocation of the argument + if ((m_spectral != NULL) && (m_spectral != spectral)) { + delete m_spectral; + } // Clone spectral model component if it exists, otherwise set pointer // to NULL @@ -462,8 +468,11 @@ void GModelSky::spectral(const GModelSpectral* spectral) ***************************************************************************/ void GModelSky::temporal(const GModelTemporal* temporal) { - // Free temporal model component - if (m_temporal != NULL) delete m_temporal; + // Free temporal model component only if it differs from current + // component. This prevents unintential deallocation of the argument + if ((m_temporal != NULL) && (m_temporal != temporal)) { + delete m_temporal; + } // Clone temporal model component if it exists, otherwise set pointer // to NULL diff --git a/src/model/GModels.cpp b/src/model/GModels.cpp index c1946a1..4868cd7 100644 --- a/src/model/GModels.cpp +++ b/src/model/GModels.cpp @@ -1,7 +1,7 @@ /*************************************************************************** * GModels.cpp - Model container class * * ----------------------------------------------------------------------- * - * copyright (C) 2009-2017 by Juergen Knoedlseder * + * copyright (C) 2009-2018 by Juergen Knoedlseder * * ----------------------------------------------------------------------- * * * * This program is free software: you can redistribute it and/or modify * @@ -332,8 +332,11 @@ GModel* GModels::set(const int& index, const GModel& model) throw GException::invalid_value(G_SET1, msg); } - // Delete any existing model - if (m_models[index] != NULL) delete m_models[index]; + // Free existing model only if it differs from current model. This + // prevents unintential deallocation of the argument + if ((m_models[index] != NULL) && (m_models[index] != &model)) { + delete m_models[index]; + } // Assign new model by cloning m_models[index] = model.clone(); @@ -379,8 +382,11 @@ GModel* GModels::set(const std::string& name, const GModel& model) throw GException::invalid_value(G_SET2, msg); } - // Delete any existing model - if (m_models[index] != NULL) delete m_models[index]; + // Free existing model only if it differs from current model. This + // prevents unintential deallocation of the argument + if ((m_models[index] != NULL) && (m_models[index] != &model)) { + delete m_models[index]; + } // Assign new model by cloning m_models[index] = model.clone(); diff --git a/src/obs/GObservation.cpp b/src/obs/GObservation.cpp index c346d79..1f60bee 100644 --- a/src/obs/GObservation.cpp +++ b/src/obs/GObservation.cpp @@ -1,7 +1,7 @@ /*************************************************************************** * GObservation.cpp - Abstract observation base class * * ----------------------------------------------------------------------- * - * copyright (C) 2008-2017 by Juergen Knoedlseder * + * copyright (C) 2008-2018 by Juergen Knoedlseder * * ----------------------------------------------------------------------- * * * * This program is free software: you can redistribute it and/or modify * @@ -769,8 +769,11 @@ double GObservation::npred_grad(const GModel& model, const GModelPar& par) const ***************************************************************************/ void GObservation::events(const GEvents& events) { - // Remove an existing event container - if (m_events != NULL) delete m_events; + // Free existing event container only if it differs from current event + // container. This prevents unintential deallocation of the argument + if ((m_events != NULL) && (m_events != &events)) { + delete m_events; + } // Clone events m_events = events.clone(); diff --git a/src/opt/GOptimizerPars.cpp b/src/opt/GOptimizerPars.cpp index 9371b07..ab9f5dd 100644 --- a/src/opt/GOptimizerPars.cpp +++ b/src/opt/GOptimizerPars.cpp @@ -1,7 +1,7 @@ /*************************************************************************** * GOptimizerPars.cpp - Optimizer parameter container class * * ----------------------------------------------------------------------- * - * copyright (C) 2009-2017 by Juergen Knoedlseder * + * copyright (C) 2009-2018 by Juergen Knoedlseder * * ----------------------------------------------------------------------- * * * * This program is free software: you can redistribute it and/or modify * @@ -346,8 +346,9 @@ GOptimizerPar* GOptimizerPars::set(const int& index, const GOptimizerPar& par) } #endif - // Delete parameter if it has been allocated - if (m_alloc[index]) { + // Free existing parameter only if it differs from current parameter. + // This prevents unintential deallocation of the argument + if ((m_alloc[index]) && (m_pars[index] != &par)) { delete m_pars[index]; } diff --git a/src/sky/GSkyMap.cpp b/src/sky/GSkyMap.cpp index 25194ad..f89557c 100644 --- a/src/sky/GSkyMap.cpp +++ b/src/sky/GSkyMap.cpp @@ -1744,8 +1744,11 @@ double GSkyMap::solidangle(const GSkyPixel& pixel) const ***************************************************************************/ void GSkyMap::projection(const GSkyProjection& proj) { - // Free any existing WCS - if (m_proj != NULL) delete m_proj; + // Free existing projection only if it differs from current projection. + // This prevents unintential deallocation of the argument + if ((m_proj != NULL) && (m_proj != &proj)) { + delete m_proj; + } // Clone input WCS m_proj = proj.clone(); diff --git a/src/sky/GSkyRegions.cpp b/src/sky/GSkyRegions.cpp index 03edba1..8dfa569 100644 --- a/src/sky/GSkyRegions.cpp +++ b/src/sky/GSkyRegions.cpp @@ -264,8 +264,11 @@ GSkyRegion* GSkyRegions::set(const int& index, const GSkyRegion& region) } #endif - // Delete any existing region - if (m_regions[index] != NULL) delete m_regions[index]; + // Free existing region only if it differs from current region. This + // prevents unintential deallocation of the argument + if ((m_regions[index] != NULL) && (m_regions[index] != ®ion)) { + delete m_regions[index]; + } // Assign new region by cloning m_regions[index] = region.clone(); diff --git a/src/test/GTestSuites.cpp b/src/test/GTestSuites.cpp index 7c828f4..fda2f76 100644 --- a/src/test/GTestSuites.cpp +++ b/src/test/GTestSuites.cpp @@ -1,7 +1,7 @@ /*************************************************************************** * GTestSuites.cpp - Test suite container class * * ----------------------------------------------------------------------- * - * copyright (C) 2012-2016 by Jean-Baptiste Cayrou * + * copyright (C) 2012-2018 by Jean-Baptiste Cayrou * * ----------------------------------------------------------------------- * * * * This program is free software: you can redistribute it and/or modify * @@ -248,8 +248,11 @@ GTestSuite* GTestSuites::set(const int& index, const GTestSuite& suite) } #endif - // Delete any existing test suite - if (m_testsuites[index] != NULL) delete m_testsuites[index]; + // Free existing test suite only if it differs from current test suite. + // This prevents unintential deallocation of the argument + if ((m_testsuites[index] != NULL) && (m_testsuites[index] != &suite)) { + delete m_testsuites[index]; + } // Assign new test suite by cloning m_testsuites[index] = suite.clone(); diff --git a/src/xml/GXmlNode.cpp b/src/xml/GXmlNode.cpp index 231621c..7d8017e 100644 --- a/src/xml/GXmlNode.cpp +++ b/src/xml/GXmlNode.cpp @@ -1,7 +1,7 @@ /*************************************************************************** * GXmlNode.cpp - Abstract XML node base class * * ----------------------------------------------------------------------- * - * copyright (C) 2010-2016 by Juergen Knoedlseder * + * copyright (C) 2010-2018 by Juergen Knoedlseder * * ----------------------------------------------------------------------- * * * * This program is free software: you can redistribute it and/or modify * @@ -253,8 +253,11 @@ GXmlNode* GXmlNode::set(const int& index, const GXmlNode& node) } #endif - // Delete any existing node - if (m_nodes[index] != NULL) delete m_nodes[index]; + // Free existing node only if it differs from current node. This + // prevents unintential deallocation of the argument + if ((m_nodes[index] != NULL) && (m_nodes[index] != &node)) { + delete m_nodes[index]; + } // Assign new child node by cloning m_nodes[index] = node.clone();