Alexander Ziegler / gammalib

Commit 6935e1da64fd25d5132644b060d228402bd612ce

Authored by Jürgen Knödlseder
1 parent 9a18f090

Fix invalid pointer problem in GLog copies (#1823)

Inline Side-by-side
Showing 4 changed files with 146 additions and 102 deletions
ChangeLog
  View file @6935e1d
... ... @@ -3,6 +3,7 @@
3 3 * Version 1.2.0 released
4 4 =========================
5 5  
  6 + Fix invalid pointer problem in GLog copies (#1823)
6 7 Fix GSkyMap::publish() bug for empty sky maps (#1835)
7 8 Fix loss of application name and version (#1835)
8 9 Add methods for logging in applications (#1828)
... ...
... ... @@ -62,6 +62,13 @@ The application name and version and now conserved in a GApplication::clear()
62 62 method call and the application is properly initialised like in the call of
63 63 a constructor (#1835).
64 64  
  65 +The GLog::copy_members() method simply copied the file pointer, hence after
  66 +a copy of a log instance there were two instances holding the same pointer
  67 +to the file, and as soon as one instance closes the log file the file
  68 +pointer in the second instance became invalid. This has been fixed by
  69 +duplicating the file descriptor and opening the log file in the copied
  70 +instance using the duplicated descriptor. This solves the problems (#1823).
  71 +
65 72  
66 73 5. Base module
67 74 --------------
... ...
src/app/GLog.cpp
  View file @6935e1d
... ... @@ -36,6 +36,7 @@
36 36 #include <iostream>
37 37 #include <sstream>
38 38 #include <fstream>
  39 +#include <unistd.h> // dup
39 40 #include "GLog.hpp"
40 41 #include "GTools.hpp"
41 42  
... ... @@ -552,7 +553,7 @@ void GLog::init_members(void)
552 553 /***********************************************************************//**
553 554 * @brief Copy class members
554 555 *
555   - * @param[in] log Object from which members which should be copied.
  556 + * @param[in] log Logger.
556 557 ***************************************************************************/
557 558 void GLog::copy_members(const GLog& log)
558 559 {
... ... @@ -563,12 +564,20 @@ void GLog::copy_members(const GLog&amp; log)
563 564 m_stderr = log.m_stderr;
564 565 m_use_date = log.m_use_date;
565 566 m_linestart = log.m_linestart;
566   - m_file = log.m_file;
567 567 m_filename = log.m_filename;
568 568 m_name = log.m_name;
569 569 m_buffer = log.m_buffer;
570 570 m_chatter = log.m_chatter;
571 571  
  572 + // Copy file pointer by duplicating the file descriptor and opening
  573 + // the duplicated file descriptor
  574 + if (log.m_file != NULL) {
  575 + m_file = fdopen(dup(fileno(log.m_file)), "a");
  576 + }
  577 + else {
  578 + m_file = NULL;
  579 + }
  580 +
572 581 // Return
573 582 return;
574 583 }
... ... @@ -820,10 +829,3 @@ void GLog::append(std::string arg)
820 829 // Return
821 830 return;
822 831 }
823   -
824   -
825   -/*==========================================================================
826   - = =
827   - = Friends =
828   - = =
829   - ==========================================================================*/
... ...
test/test_GApplication.py
  View file @6935e1d
1 1 # ==========================================================================
2   -# This module performs unit tests for the GammaLib application module.
  2 +# This module performs unit tests for the GammaLib application module
3 3 #
4   -# Copyright (C) 2012-2015 Juergen Knoedlseder
  4 +# Copyright (C) 2012-2016 Juergen Knoedlseder
5 5 #
6 6 # This program is free software: you can redistribute it and/or modify
7 7 # it under the terms of the GNU General Public License as published by
... ... @@ -25,150 +25,184 @@ import gammalib
25 25 # ========================================== #
26 26 class Test(gammalib.GPythonTestSuite):
27 27 """
28   - Test class for GammaLib application module.
  28 + Test class for GammaLib application module
29 29 """
30 30 # Constructor
31 31 def __init__(self):
32 32 """
33   - Constructor.
  33 + Constructor
34 34 """
35 35 # Call base class constructor
36 36 gammalib.GPythonTestSuite.__init__(self)
37 37  
38 38 # Set members
39   - self.logfile = "test_logger.log"
  39 + self._logfile = 'test_logger.log'
40 40  
41 41 # Return
42 42 return
43 43  
44   - # Set test functions
  44 +
  45 + # Private methods
  46 + def _check_log_file(self, ref, calls):
  47 + """
  48 + Check log file versus a reference
  49 +
  50 + Parameters
  51 + ----------
  52 + ref : list of str
  53 + Reference lines
  54 + calls : list of str
  55 + Methods that are tested
  56 + """
  57 + # Open log file, read all line and close file
  58 + file = open(self._logfile, 'r')
  59 + lines = file.readlines()
  60 + file.close()
  61 +
  62 + # Check all lines versus reference. There is a special kluge in case
  63 + # that the reference is "true\n" as in some Python/SWIG version this
  64 + # is displayed by "1\n". Both variants are tested to avoid version
  65 + # dependent test results.
  66 + for i, line in enumerate(lines):
  67 + if ref[i] == 'true\n':
  68 + self.test_assert(line == ref[i] or line == '1\n',
  69 + 'Check '+calls[i]+': '+line)
  70 + else:
  71 + self.test_value(line, ref[i], 'Check '+calls[i]+': '+line)
  72 +
  73 + # Return
  74 + return
  75 +
  76 +
  77 + # Public methods
45 78 def set(self):
46 79 """
47   - Set all test functions.
  80 + Set all test functions
48 81 """
49 82 # Set test name
50   - self.name("app")
  83 + self.name('app')
51 84  
52 85 # Append tests
53   - self.append(self.test_log, "Test GLog")
54   - self.append(self.test_pars, "Test GApplicationPars")
  86 + self.append(self.test_log, 'Test GLog')
  87 + self.append(self.test_pars, 'Test GApplicationPars')
55 88  
56 89 # Return
57 90 return
58 91  
59   - # Test GLog
60 92 def test_log(self):
61 93 """
62   - Test GLog.
  94 + Test GLog
63 95 """
64 96 # Allocate logger
65   - log = gammalib.GLog(self.logfile, True)
  97 + log = gammalib.GLog(self._logfile, True)
66 98  
67 99 # Test print methods
68   - log("Test __call__(std::string)")
69   - log("\n")
  100 + log('Test __call__(std::string)\n')
70 101 log(True)
71   - log("\n")
  102 + log('\n')
72 103 log(41)
73   - log("\n")
  104 + log('\n')
74 105 log(3.1415)
75   - log("\n")
76   - log.parformat("This is a parameter")
77   - log("\n")
78   - log.toupper("upper case")
79   - log("\n")
80   - log.tolower("LOWER CASE")
81   - log("\n")
82   - log.fill("Higgs", 3)
83   - log("\n")
84   - log.left("Left", 10)
85   - log("\n")
86   - log.right("Right", 10)
87   - log("\n")
88   - log.centre("Centre", 10)
89   - log("\n")
90   - log.header0("Header 0")
91   - log("\n")
92   - log.header1("Header 1")
93   - log.header2("Header 2")
94   - log.header3("Header 3")
  106 + log('\n')
  107 + log.parformat('This is a parameter')
  108 + log('\n')
  109 + log.toupper('upper case')
  110 + log('\n')
  111 + log.tolower('LOWER CASE')
  112 + log('\n')
  113 + log.fill('Higgs', 3)
  114 + log('\n')
  115 + log.left('Left', 10)
  116 + log('\n')
  117 + log.right('Right', 10)
  118 + log('\n')
  119 + log.centre('Centre', 10)
  120 + log('\n')
  121 + log.header0('Header 0')
  122 + log('\n')
  123 + log.header1('Header 1')
  124 + log.header2('Header 2')
  125 + log.header3('Header 3')
95 126  
96 127 # Close logger
97 128 log.close()
98 129  
99 130 # Set reference
100 131 ref = []
101   - ref.append("Test __call__(std::string)\n")
102   - #ref.append("1\n") # Old swig version
103   - ref.append("true\n") # New swig version (3.x.y)
104   - ref.append("41\n")
105   - ref.append("3.1415\n")
106   - ref.append(" This is a parameter .......: \n")
107   - ref.append("UPPER CASE\n")
108   - ref.append("lower case\n")
109   - ref.append("HiggsHiggsHiggs\n")
110   - ref.append("Left \n")
111   - ref.append(" Right\n")
112   - ref.append(" Centre \n")
113   - ref.append("\n")
114   - ref.append("+==========+\n")
115   - ref.append("| Header 1 |\n")
116   - ref.append("+==========+\n")
117   - ref.append("+----------+\n")
118   - ref.append("| Header 2 |\n")
119   - ref.append("+----------+\n")
120   - ref.append("=== Header 3 ===\n")
  132 + ref.append('Test __call__(std::string)\n')
  133 + ref.append('true\n') # New swig version (3.x.y)
  134 + ref.append('41\n')
  135 + ref.append('3.1415\n')
  136 + ref.append(' This is a parameter .......: \n')
  137 + ref.append('UPPER CASE\n')
  138 + ref.append('lower case\n')
  139 + ref.append('HiggsHiggsHiggs\n')
  140 + ref.append('Left \n')
  141 + ref.append(' Right\n')
  142 + ref.append(' Centre \n')
  143 + ref.append('\n')
  144 + ref.append('+==========+\n')
  145 + ref.append('| Header 1 |\n')
  146 + ref.append('+==========+\n')
  147 + ref.append('+----------+\n')
  148 + ref.append('| Header 2 |\n')
  149 + ref.append('+----------+\n')
  150 + ref.append('=== Header 3 ===\n')
121 151  
122 152 # Set calls for error messages
123 153 calls = []
124   - calls.append("__call__(std::string)")
125   - calls.append("__call__(bool)")
126   - calls.append("__call__(int)")
127   - calls.append("__call__(double)")
128   - calls.append("parformat()")
129   - calls.append("toupper()")
130   - calls.append("tolower()")
131   - calls.append("fill()")
132   - calls.append("left()")
133   - calls.append("right()")
134   - calls.append("centre()")
135   - calls.append("header0()")
136   - calls.append("header1()")
137   - calls.append("header1()")
138   - calls.append("header1()")
139   - calls.append("header2()")
140   - calls.append("header2()")
141   - calls.append("header2()")
142   - calls.append("header3()")
143   -
144   - # Check file
145   - file = open(self.logfile, "r")
146   - lines = file.readlines()
147   - file.close()
148   - for i, line in enumerate(lines):
149   - if ref[i] == "true\n":
150   - self.test_assert(line == ref[i] or line == "1\n",
151   - "Test "+calls[i]+": "+line)
152   - else:
153   - self.test_assert(line == ref[i], "Test "+calls[i]+": "+line)
  154 + calls.append('__call__(std::string)')
  155 + calls.append('__call__(bool)')
  156 + calls.append('__call__(int)')
  157 + calls.append('__call__(double)')
  158 + calls.append('parformat()')
  159 + calls.append('toupper()')
  160 + calls.append('tolower()')
  161 + calls.append('fill()')
  162 + calls.append('left()')
  163 + calls.append('right()')
  164 + calls.append('centre()')
  165 + calls.append('header0()')
  166 + calls.append('header1()')
  167 + calls.append('header1()')
  168 + calls.append('header1()')
  169 + calls.append('header2()')
  170 + calls.append('header2()')
  171 + calls.append('header2()')
  172 + calls.append('header3()')
  173 +
  174 + # Check log file
  175 + self._check_log_file(ref, calls)
  176 +
  177 + # Create copy of logger
  178 + cpy_log = log.copy()
  179 +
  180 + # Print one more line into copy
  181 + cpy_log.header3('Header 3bis')
  182 +
  183 + # Append reference and call for error message
  184 + ref.append('=== Header 3bis ===\n')
  185 + calls.append('header3()')
  186 +
  187 + # Check log file again
  188 + self._check_log_file(ref, calls)
154 189  
155 190 # Return
156 191 return
157 192  
158   - # Test GApplicationPars
159 193 def test_pars(self):
160 194 """
161   - Test GApplicationPars.
  195 + Test GApplicationPars
162 196 """
163 197 # Test GApplicationPars constructor with bad filename.
164   - self.test_try("Test GApplicationPars constructor with bad filename")
  198 + self.test_try('Test GApplicationPars constructor with bad filename')
165 199 try:
166   - pars = gammalib.GApplicationPars("testme.par")
167   - self.test_try_failure("Exception not thrown.")
  200 + pars = gammalib.GApplicationPars('testme.par')
  201 + self.test_try_failure('Exception not thrown')
168 202 except:
169 203 self.test_try_success()
170 204 else:
171   - self.test_try_failure("This should never happen.")
  205 + self.test_try_failure('This should never happen')
172 206  
173 207 # Return
174 208 return
... ...