Hiu Truls et al, Truls Martin Larsen <t.m.larsen@fys.uio.no> wrote concerning Brat 2-5-3 [Tue, 01 Oct 2002 13:52:35 +0200] ---------------------------------------------------------------------- > Hi all, > > some minor but maybe important changes. Two possible array overflows: > > In BrDigtizeTPC::Event() method three arrays had the possibility to be > filled with more entries than the array size. The for loops were changed. This module seems to suffer from Fortrantitis: Many variables declared in one function that spans several hundred lines of code, heavy use of arrays, and so on. When dealing with things like that, one should try to cache the memory as one best can. An obvious solution is to replace all the arrays with one container of sorts, containing objects of a utility class. The container should be a cache, meaning it doesn't delete memory unnecessarily (see the attached file). > In BrDetectorParmasBase::ReadASCIIFile() the variable system_param_file > was an Char_t[64] array which was a bitt to small to hold the entire > path to the BrDetectorParameters.txt Changed it to Char_t[256]. Why is it a Char_t array in the first place? It should be a TString or a std::string. > Tagged brat since these were possible memory leaks (I guess it is > better to tag one time to often than one time to little...) Oh yes - and I wish everyone would follow that rational. Recently I compiled BRAT 2.4.7 (as it is our current `pro' release of BRAT) and discovered it didn't even compile! <sarcasm>Now that's nice isn't it? - out production BRAT doesn't compile!</sarcasm>. BTW, iff BRAT-2.4.7 is too old and BRAT 2.5.3 is more or less stable (at least compiles!), then that should become our `production' version. As outlined elsewhere (numerous emails and I believe also in `The Guide') this is accomplished by starting a new development cycle, which is done by incrementing the minor version number and zeroing the revision number. > > They were both found using insure++ Also check out the ROOT utility `memprobe' and standalone utility `LeakTracer' [1] for detecting memory leaks GCC compiled C++. [1] http://www.andreasen.org/LeakTracer/ Yours, ____ | Christian Holm Christensen |_| | ------------------------------------------------------------- | | Address: Sankt Hansgade 23, 1. th. Phone: (+45) 35 35 96 91 _| DK-2200 Copenhagen N Cell: (+45) 24 61 85 91 _| Denmark Office: (+45) 353 25 305 ____| Email: cholm@nbi.dk Web: www.nbi.dk/~cholm | | // -*- mode: c++ -*- // // $Id: Foo.hh,v 1.1.1.1 2002/08/08 03:37:21 cholm Exp $ // // std::cache // Copyright (C) 2002 Christian Holm Christensen <cholm@nbi.dk> // // This library is free software; you can redistribute it and/or // modify it under the terms of the GNU Lesser General Public License // as published by the Free Software Foundation; either version 2.1 // of the License, or (at your option) any later version. // // This library is distributed in the hope that it will be useful, // but WITHOUT ANY WARRANTY; without even the implied warranty of // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU // Lesser General Public License for more details. // // You should have received a copy of the GNU Lesser General Public // License along with this library; if not, write to the Free // Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA // 02111-1307 USA // #ifndef __CACHE__ #define __CACHE__ #ifndef __VECTOR__ #include <vector> #endif namespace std { /** Cached memory container class. <b>Requirements of T</b> <ul> <li></li> </ul> <b>Requirements of S</b> <ul> <li>Should be a STL container of T* objects</li> <li>Is subscriptable by operator[](iterator)</li> </ul> @code std::cache<int> c(5); for (int i = 0; i < 10; i++) new(cache.address(i)) int(i+1); for (std::cache<int>::iterator i = c.begin(); i != c.end(); i++) std::cout << *(*i) << std::endl; c.clear(); for (int i = 0; i < 10; i++) new(cache.address(i)) int(i+1); for (std::cache<int>::iterator i = c.begin(); i != c.end(); i++) std::cout << *(*i) << std::endl; @endcode */ template <class T, class S=std::vector<T*> > class cache { private: /// The actual container S _container; /// Cache of allocated memory S _cache; public: /// Type of container typedef S container_type; /// Type of the contained objects typedef typename S::value_type value_type; /// Type of the size typedef typename S::size_type size_type; /// Type of references to contained objects typedef typename S::reference reference; /// Type of constant references to contained objects typedef typename S::const_reference const_reference; /// Type of an iterator over the elements typedef typename S::iterator iterator; /// Type of an constant iterator over the elements typedef typename S::const_iterator const_iterator; /** Default constructor. @param size The initial size of the cache. */ cache(size_type size=1) : _container(size), _cache(size) {} /** Copy constructor. @param seq Sequence to initialise cache from. */ cache(const container_type& seq) : _container(seq), _cache(seq) {} /** Destructor. This free's all memory allocated for the contained objects. Hence the cache is the owner of not only the pointers to the objects, but also of the actual objects. */ virtual ~cache(); /** Iterator pointing at front of cache. @return Iterator pointing at front of cache. */ iterator begin() { return _container.begin(); } /** Iterator pointing at front of cache. @return Iterator pointing at front of cache. */ const iterator begin() const { return _container.begin(); } /** Iterator pointing at end of cache. @return Iterator pointing at end of cache. */ iterator end() { return _container.end(); } /** Iterator pointing at end of cache. @return Iterator pointing at end of cache. */ const iterator end() const { return _container.end(); } /** Get the size of the cache. @return size of the cache. */ size_type size() const { return _container.size(); } /** Test if cache is empty. @return true if empty, false otherwise. */ bool empty() const { return _container.empty(); } /** Reference to the front element of the cache. @return reference to front element of the cache. */ reference front() { return _container.front(); } /** Reference to the front element of the cache. @return reference to front element of the cache. */ const_reference front() const { return _container.front(); } /** Reference to the back element of the cache. @return reference to back element of the cache. */ reference back() { return _container.back(); } /** Reference to the back element of the cache. @return reference to back element of the cache. */ const_reference back() const { return _container.back(); } /** Element access. Get the address of an element. If the element isn't in the cache i.e., pos is out of range, then the cache is expanded upto pos elements. @param pos The position of element address to access. */ reference address(size_type pos); /** Remove an element. @param pos Position of objet to remove. */ void erase(iterator pos) { _container.erase(pos); } /** Clear the array. */ void clear() { _container.clear(); } /** element access. @param pos element position to get. */ reference operator[](size_type pos) { return _container[pos]; } /** element access. @param pos element position to get. */ const_reference operator[](size_type pos) const { return _container[pos]; } }; //__________________________________________________________________ template <class T, class S> cache<T,S>::~cache() { for (iterator i = _cache.begin(); i != _cache.end(); i++) { value_type e = (*i); delete e; } _cache.clear(); _container.clear(); } //__________________________________________________________________ template <class T, class S> cache<T,S>::reference cache<T,S>::address(cache<T,S>::size_type pos) { if (pos >= _container.size()) _container.resize(pos+1); if (pos >= _cache.size()) _cache.resize(pos+1); if (!_cache[pos]) _cache[pos] = new T; return _container[pos] = _cache[pos]; } } #endif
This archive was generated by hypermail 2b30 : Tue Oct 01 2002 - 15:26:02 EDT