Andreas Specovius / gammalib

Commit 0c962213bc4098d3fee6de63116b38b89414b4d8

Authored by Andreas Specovius
2 parents 3c59f823 0be8dd0c

Merge branch 'devel' of https://cta-gitlab.irap.omp.eu/gammalib/gammalib into 00… 

…00-more-flexible-fits-reader
Inline Side-by-side
Showing 15 changed files with 110 additions and 57 deletions
ChangeLog
  View file @0c96221
... ... @@ -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 @0c96221
... ... @@ -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 @0c96221
... ... @@ -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 @0c96221
... ... @@ -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();
... ...
pyext/GNdarray.i
  View file @0c96221
... ... @@ -108,21 +108,34 @@ public:
108 108 return ((*self)(index));
109 109 }
110 110 void __setitem__(int GTuple[], const double& value) {
111   - if (GTuple[0] != self->dim()) {
112   - throw GException::invalid_value("__setitem__(int)", "Invalid "
113   - "access of "+gammalib::str(self->dim())+"-dimensional "
114   - "array with "+gammalib::str(GTuple[0])+"-dimensional "
115   - "access operator.");
  111 + // Handle first the special case of a single index. In Python we want
  112 + // to support accessing a n-dimensional array with a single index
  113 + if (GTuple[0] == 1) {
  114 + if (GTuple[1] >= 0 && GTuple[1] < self->size()) {
  115 + (*self)(GTuple[1]) = value;
  116 + }
  117 + else {
  118 + throw GException::out_of_range("__ssetitem__(int)", "Array "
  119 + "index", GTuple[1], self->size());
  120 + }
116 121 }
117   - int index = 0;
118   - for (size_t i = 0; i < self->shape().size(); ++i) {
119   - if (GTuple[i+1] < 0 || GTuple[i+1] >= self->shape()[i]) {
120   - throw GException::out_of_range("__setitem__(int)", "Dimension "+
121   - gammalib::str(i)+" index", GTuple[i+1], self->shape()[i]);
  122 + else {
  123 + if (GTuple[0] != self->dim()) {
  124 + throw GException::invalid_value("__setitem__(int)", "Invalid "
  125 + "access of "+gammalib::str(self->dim())+"-dimensional "
  126 + "array with "+gammalib::str(GTuple[0])+"-dimensional "
  127 + "access operator.");
122 128 }
123   - index += self->strides()[i] * GTuple[i+1];
  129 + int index = 0;
  130 + for (size_t i = 0; i < self->shape().size(); ++i) {
  131 + if (GTuple[i+1] < 0 || GTuple[i+1] >= self->shape()[i]) {
  132 + throw GException::out_of_range("__setitem__(int)", "Dimension "+
  133 + gammalib::str(i)+" index", GTuple[i+1], self->shape()[i]);
  134 + }
  135 + index += self->strides()[i] * GTuple[i+1];
  136 + }
  137 + (*self)(index) = value;
124 138 }
125   - (*self)(index) = value;
126 139 }
127 140 GNdarray __add__(const GNdarray &a) {
128 141 return (*self) + a;
... ...
src/fits/GFits.cpp
  View file @0c96221
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 @0c96221
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 *
... ... @@ -338,8 +338,11 @@ GFitsTableCol* GFitsTable::set(const int&amp; colnum, const GFitsTableCol&amp; column)
338 338 throw GException::out_of_range(G_SET1, "Column number", colnum, ncols());
339 339 }
340 340  
341   - // Free existing column
342   - if (m_columns[colnum] != NULL) delete m_columns[colnum];
  341 + // Free existing column only if it differs from current column. This
  342 + // prevents unintential deallocation of the argument
  343 + if ((m_columns[colnum] != NULL) && (m_columns[colnum] != &column)) {
  344 + delete m_columns[colnum];
  345 + }
343 346  
344 347 // Clone column
345 348 m_columns[colnum] = column.clone();
... ... @@ -398,12 +401,6 @@ GFitsTableCol* GFitsTable::set(const std::string&amp; colname,
398 401 ***************************************************************************/
399 402 GFitsTableCol* GFitsTable::insert(int colnum, const GFitsTableCol& column)
400 403 {
401   - // Make sure that 'colnum' is valid. Since we add one column the total
402   - // number of columns at return will be m_cols+1, hence the column
403   - // index is comprised between [0,m_cols]
404   - //if (colnum < 0) colnum = 0;
405   - //if (colnum > m_cols) colnum = m_cols;
406   -
407 404 // Check if column number is valid
408 405 if (colnum < 0 || colnum > ncols()) {
409 406 throw GException::out_of_range(G_INSERT1, "Column number", colnum, ncols()+1);
... ...
src/model/GModelSky.cpp
  View file @0c96221
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 @0c96221
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 @0c96221
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 @0c96221
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 @0c96221
... ... @@ -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 @0c96221
... ... @@ -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 @0c96221
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 @0c96221
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();
... ...