Rolf Buehler / gammalib

Commit 0be8dd0c21f69690851b35431575b03a4e90ef63

Authored by Jürgen Knödlseder
2 parents 8bda5654 ad650400

Merge branch '2340-prevent-unintentional-deallocation' into integration 

# Conflicts:
#	ChangeLog
#	NEWS
Inline Side-by-side
Showing 14 changed files with 80 additions and 43 deletions
ChangeLog
  View file @0be8dd0
... ... @@ -7,11 +7,12 @@
7 7 Add GCTABackground::rate_ebin() method (#2308)
8 8  
9 9  
10   -2018-02-13
  10 +2018-02-15
11 11  
12 12 * Bug fix version 1.5.1 released
13 13 ================================
14 14  
  15 + Prevent unintentional deallocation of arguments (#2340).
15 16 Correct sky map smoothing.
16 17  
17 18  
... ...
doc/source/admin/release_history/1.5.1.rst
  View file @0be8dd0
... ... @@ -10,6 +10,8 @@ GammaLib 1.5.1 is a bug fix release for GammaLib release 1.5.
10 10  
11 11 The following bugs were fixed:
12 12  
  13 +* [`2340 <https://cta-redmine.irap.omp.eu/issues/2340>`_] -
  14 + Prevent unintentional deallocation of arguments
13 15 * Correct sky map smoothing
14 16  
15 17  
... ...
inst/cta/src/GCTAObservation.cpp
  View file @0be8dd0
... ... @@ -335,8 +335,11 @@ void GCTAObservation::response(const GResponse&amp; rsp)
335 335 throw GException::invalid_argument(G_RESPONSE_SET, msg);
336 336 }
337 337  
338   - // Free response
339   - if (m_response != NULL) delete m_response;
  338 + // Free existing response only if it differs from current response. This
  339 + // prevents unintential deallocation of the argument
  340 + if ((m_response != NULL) && (m_response != &rsp)) {
  341 + delete m_response;
  342 + }
340 343  
341 344 // Clone response function
342 345 m_response = ptr->clone();
... ...
inst/cta/src/GCTAOnOffObservation.cpp
  View file @0be8dd0
... ... @@ -575,8 +575,11 @@ void GCTAOnOffObservation::response(const GResponse&amp; rsp)
575 575 throw GException::invalid_argument(G_RESPONSE_SET, msg);
576 576 }
577 577  
578   - // Free response
579   - if (m_response != NULL) delete m_response;
  578 + // Free existing response only if it differs from current response. This
  579 + // prevents unintential deallocation of the argument
  580 + if ((m_response != NULL) && (m_response != &rsp)) {
  581 + delete m_response;
  582 + }
580 583  
581 584 // Clone response function
582 585 m_response = ptr->clone();
... ...
src/fits/GFits.cpp
  View file @0be8dd0
1 1 /***************************************************************************
2 2 * GFits.cpp - FITS file access class *
3 3 * ----------------------------------------------------------------------- *
4   - * copyright (C) 2008-2017 by Juergen Knoedlseder *
  4 + * copyright (C) 2008-2018 by Juergen Knoedlseder *
5 5 * ----------------------------------------------------------------------- *
6 6 * *
7 7 * This program is free software: you can redistribute it and/or modify *
... ... @@ -601,8 +601,11 @@ GFitsHDU* GFits::set(const int&amp; extno, const GFitsHDU&amp; hdu)
601 601 throw GException::invalid_argument(G_SET1, msg);
602 602 }
603 603  
604   - // Delete any existing HDU
605   - if (m_hdu[extno] != NULL) delete m_hdu[extno];
  604 + // Free existing HDU only if it differs from current HDU. This prevents
  605 + // unintential deallocation of the argument
  606 + if ((m_hdu[extno] != NULL) && (m_hdu[extno] != &hdu)) {
  607 + delete m_hdu[extno];
  608 + }
606 609  
607 610 // Assign new HDU by cloning
608 611 m_hdu[extno] = hdu.clone();
... ...
src/fits/GFitsTable.cpp
  View file @0be8dd0
1 1 /***************************************************************************
2 2 * GFitsTable.cpp - FITS table base class *
3 3 * ----------------------------------------------------------------------- *
4   - * copyright (C) 2008-2017 by Juergen Knoedlseder *
  4 + * copyright (C) 2008-2018 by Juergen Knoedlseder *
5 5 * ----------------------------------------------------------------------- *
6 6 * *
7 7 * This program is free software: you can redistribute it and/or modify *
... ... @@ -401,12 +401,6 @@ GFitsTableCol* GFitsTable::set(const std::string&amp; colname,
401 401 ***************************************************************************/
402 402 GFitsTableCol* GFitsTable::insert(int colnum, const GFitsTableCol& column)
403 403 {
404   - // Make sure that 'colnum' is valid. Since we add one column the total
405   - // number of columns at return will be m_cols+1, hence the column
406   - // index is comprised between [0,m_cols]
407   - //if (colnum < 0) colnum = 0;
408   - //if (colnum > m_cols) colnum = m_cols;
409   -
410 404 // Check if column number is valid
411 405 if (colnum < 0 || colnum > ncols()) {
412 406 throw GException::out_of_range(G_INSERT1, "Column number", colnum, ncols()+1);
... ...
src/model/GModelSky.cpp
  View file @0be8dd0
1 1 /***************************************************************************
2 2 * GModelSky.cpp - Sky model class *
3 3 * ----------------------------------------------------------------------- *
4   - * copyright (C) 2011-2017 by Juergen Knoedlseder *
  4 + * copyright (C) 2011-2018 by Juergen Knoedlseder *
5 5 * ----------------------------------------------------------------------- *
6 6 * *
7 7 * This program is free software: you can redistribute it and/or modify *
... ... @@ -411,8 +411,11 @@ GModelSky* GModelSky::clone(void) const
411 411 ***************************************************************************/
412 412 void GModelSky::spatial(const GModelSpatial* spatial)
413 413 {
414   - // Free spatial model component
415   - if (m_spatial != NULL) delete m_spatial;
  414 + // Free spatial model component only if it differs from current
  415 + // component. This prevents unintential deallocation of the argument
  416 + if ((m_spatial != NULL) && (m_spatial != spatial)) {
  417 + delete m_spatial;
  418 + }
416 419  
417 420 // Clone spatial model component if it exists, otherwise set pointer
418 421 // to NULL
... ... @@ -438,8 +441,11 @@ void GModelSky::spatial(const GModelSpatial* spatial)
438 441 ***************************************************************************/
439 442 void GModelSky::spectral(const GModelSpectral* spectral)
440 443 {
441   - // Free spectral model component
442   - if (m_spectral != NULL) delete m_spectral;
  444 + // Free spectral model component only if it differs from current
  445 + // component. This prevents unintential deallocation of the argument
  446 + if ((m_spectral != NULL) && (m_spectral != spectral)) {
  447 + delete m_spectral;
  448 + }
443 449  
444 450 // Clone spectral model component if it exists, otherwise set pointer
445 451 // to NULL
... ... @@ -462,8 +468,11 @@ void GModelSky::spectral(const GModelSpectral* spectral)
462 468 ***************************************************************************/
463 469 void GModelSky::temporal(const GModelTemporal* temporal)
464 470 {
465   - // Free temporal model component
466   - if (m_temporal != NULL) delete m_temporal;
  471 + // Free temporal model component only if it differs from current
  472 + // component. This prevents unintential deallocation of the argument
  473 + if ((m_temporal != NULL) && (m_temporal != temporal)) {
  474 + delete m_temporal;
  475 + }
467 476  
468 477 // Clone temporal model component if it exists, otherwise set pointer
469 478 // to NULL
... ...
src/model/GModels.cpp
  View file @0be8dd0
1 1 /***************************************************************************
2 2 * GModels.cpp - Model container class *
3 3 * ----------------------------------------------------------------------- *
4   - * copyright (C) 2009-2017 by Juergen Knoedlseder *
  4 + * copyright (C) 2009-2018 by Juergen Knoedlseder *
5 5 * ----------------------------------------------------------------------- *
6 6 * *
7 7 * This program is free software: you can redistribute it and/or modify *
... ... @@ -332,8 +332,11 @@ GModel* GModels::set(const int&amp; index, const GModel&amp; model)
332 332 throw GException::invalid_value(G_SET1, msg);
333 333 }
334 334  
335   - // Delete any existing model
336   - if (m_models[index] != NULL) delete m_models[index];
  335 + // Free existing model only if it differs from current model. This
  336 + // prevents unintential deallocation of the argument
  337 + if ((m_models[index] != NULL) && (m_models[index] != &model)) {
  338 + delete m_models[index];
  339 + }
337 340  
338 341 // Assign new model by cloning
339 342 m_models[index] = model.clone();
... ... @@ -379,8 +382,11 @@ GModel* GModels::set(const std::string&amp; name, const GModel&amp; model)
379 382 throw GException::invalid_value(G_SET2, msg);
380 383 }
381 384  
382   - // Delete any existing model
383   - if (m_models[index] != NULL) delete m_models[index];
  385 + // Free existing model only if it differs from current model. This
  386 + // prevents unintential deallocation of the argument
  387 + if ((m_models[index] != NULL) && (m_models[index] != &model)) {
  388 + delete m_models[index];
  389 + }
384 390  
385 391 // Assign new model by cloning
386 392 m_models[index] = model.clone();
... ...
src/obs/GObservation.cpp
  View file @0be8dd0
1 1 /***************************************************************************
2 2 * GObservation.cpp - Abstract observation base class *
3 3 * ----------------------------------------------------------------------- *
4   - * copyright (C) 2008-2017 by Juergen Knoedlseder *
  4 + * copyright (C) 2008-2018 by Juergen Knoedlseder *
5 5 * ----------------------------------------------------------------------- *
6 6 * *
7 7 * This program is free software: you can redistribute it and/or modify *
... ... @@ -769,8 +769,11 @@ double GObservation::npred_grad(const GModel&amp; model, const GModelPar&amp; par) const
769 769 ***************************************************************************/
770 770 void GObservation::events(const GEvents& events)
771 771 {
772   - // Remove an existing event container
773   - if (m_events != NULL) delete m_events;
  772 + // Free existing event container only if it differs from current event
  773 + // container. This prevents unintential deallocation of the argument
  774 + if ((m_events != NULL) && (m_events != &events)) {
  775 + delete m_events;
  776 + }
774 777  
775 778 // Clone events
776 779 m_events = events.clone();
... ...
src/opt/GOptimizerPars.cpp
  View file @0be8dd0
1 1 /***************************************************************************
2 2 * GOptimizerPars.cpp - Optimizer parameter container class *
3 3 * ----------------------------------------------------------------------- *
4   - * copyright (C) 2009-2017 by Juergen Knoedlseder *
  4 + * copyright (C) 2009-2018 by Juergen Knoedlseder *
5 5 * ----------------------------------------------------------------------- *
6 6 * *
7 7 * This program is free software: you can redistribute it and/or modify *
... ... @@ -346,8 +346,9 @@ GOptimizerPar* GOptimizerPars::set(const int&amp; index, const GOptimizerPar&amp; par)
346 346 }
347 347 #endif
348 348  
349   - // Delete parameter if it has been allocated
350   - if (m_alloc[index]) {
  349 + // Free existing parameter only if it differs from current parameter.
  350 + // This prevents unintential deallocation of the argument
  351 + if ((m_alloc[index]) && (m_pars[index] != &par)) {
351 352 delete m_pars[index];
352 353 }
353 354  
... ...
src/sky/GSkyMap.cpp
  View file @0be8dd0
... ... @@ -1744,8 +1744,11 @@ double GSkyMap::solidangle(const GSkyPixel&amp; pixel) const
1744 1744 ***************************************************************************/
1745 1745 void GSkyMap::projection(const GSkyProjection& proj)
1746 1746 {
1747   - // Free any existing WCS
1748   - if (m_proj != NULL) delete m_proj;
  1747 + // Free existing projection only if it differs from current projection.
  1748 + // This prevents unintential deallocation of the argument
  1749 + if ((m_proj != NULL) && (m_proj != &proj)) {
  1750 + delete m_proj;
  1751 + }
1749 1752  
1750 1753 // Clone input WCS
1751 1754 m_proj = proj.clone();
... ...
src/sky/GSkyRegions.cpp
  View file @0be8dd0
... ... @@ -264,8 +264,11 @@ GSkyRegion* GSkyRegions::set(const int&amp; index, const GSkyRegion&amp; region)
264 264 }
265 265 #endif
266 266  
267   - // Delete any existing region
268   - if (m_regions[index] != NULL) delete m_regions[index];
  267 + // Free existing region only if it differs from current region. This
  268 + // prevents unintential deallocation of the argument
  269 + if ((m_regions[index] != NULL) && (m_regions[index] != &region)) {
  270 + delete m_regions[index];
  271 + }
269 272  
270 273 // Assign new region by cloning
271 274 m_regions[index] = region.clone();
... ...
src/test/GTestSuites.cpp
  View file @0be8dd0
1 1 /***************************************************************************
2 2 * GTestSuites.cpp - Test suite container class *
3 3 * ----------------------------------------------------------------------- *
4   - * copyright (C) 2012-2016 by Jean-Baptiste Cayrou *
  4 + * copyright (C) 2012-2018 by Jean-Baptiste Cayrou *
5 5 * ----------------------------------------------------------------------- *
6 6 * *
7 7 * This program is free software: you can redistribute it and/or modify *
... ... @@ -248,8 +248,11 @@ GTestSuite* GTestSuites::set(const int&amp; index, const GTestSuite&amp; suite)
248 248 }
249 249 #endif
250 250  
251   - // Delete any existing test suite
252   - if (m_testsuites[index] != NULL) delete m_testsuites[index];
  251 + // Free existing test suite only if it differs from current test suite.
  252 + // This prevents unintential deallocation of the argument
  253 + if ((m_testsuites[index] != NULL) && (m_testsuites[index] != &suite)) {
  254 + delete m_testsuites[index];
  255 + }
253 256  
254 257 // Assign new test suite by cloning
255 258 m_testsuites[index] = suite.clone();
... ...
src/xml/GXmlNode.cpp
  View file @0be8dd0
1 1 /***************************************************************************
2 2 * GXmlNode.cpp - Abstract XML node base class *
3 3 * ----------------------------------------------------------------------- *
4   - * copyright (C) 2010-2016 by Juergen Knoedlseder *
  4 + * copyright (C) 2010-2018 by Juergen Knoedlseder *
5 5 * ----------------------------------------------------------------------- *
6 6 * *
7 7 * This program is free software: you can redistribute it and/or modify *
... ... @@ -253,8 +253,11 @@ GXmlNode* GXmlNode::set(const int&amp; index, const GXmlNode&amp; node)
253 253 }
254 254 #endif
255 255  
256   - // Delete any existing node
257   - if (m_nodes[index] != NULL) delete m_nodes[index];
  256 + // Free existing node only if it differs from current node. This
  257 + // prevents unintential deallocation of the argument
  258 + if ((m_nodes[index] != NULL) && (m_nodes[index] != &node)) {
  259 + delete m_nodes[index];
  260 + }
258 261  
259 262 // Assign new child node by cloning
260 263 m_nodes[index] = node.clone();
... ...