Browse Source

[trac1010] address review comments

Jelte Jansen 14 years ago
parent
commit
b5cfd5e541
3 changed files with 12 additions and 5 deletions
  1. 1 1
      src/lib/config/ccsession.cc
  2. 2 2
      src/lib/config/ccsession.h
  3. 9 2
      src/lib/python/isc/log/log.cc

+ 1 - 1
src/lib/config/ccsession.cc

@@ -247,7 +247,7 @@ readLoggersConf(std::vector<isc::log::LoggerSpecification>& specs,
 } // end anonymous namespace
 
 void
-default_logconfig_handler(const std::string&n,
+default_logconfig_handler(const std::string& module_name,
                           ConstElementPtr new_config,
                           const ConfigData& config_data) {
     config_data.getModuleSpec().validateConfig(new_config, true);

+ 2 - 2
src/lib/config/ccsession.h

@@ -364,12 +364,12 @@ private:
 /// LoggerManager and passing the settings as specified in the given
 /// configuration update.
 ///
-/// \param n The name of the module
+/// \param module_name The name of the module
 /// \param new_config The modified configuration values
 /// \param config_data The full config data for the (remote) logging
 ///                    module.
 void
-default_logconfig_handler(const std::string&n,
+default_logconfig_handler(const std::string& module_name,
                           isc::data::ConstElementPtr new_config,
                           const ConfigData& config_data);
 

+ 9 - 2
src/lib/python/isc/log/log.cc

@@ -38,6 +38,11 @@ namespace {
 MessageDictionary* testDictionary = NULL;
 
 // To propagate python exceptions trough our code
+// This exception is used to signal to the calling function that a
+// proper Python Exception has already been set, and the caller
+// should now return NULL.
+// Since it is only used internally, and should not pass any
+// information itself, is is not derived from std::exception
 class InternalError {};
 
 PyObject*
@@ -206,6 +211,8 @@ isc::data::ConstElementPtr PyObjectToElement(PyObject* obj) {
     } else if (obj == Py_None) {
         return isc::data::ElementPtr();
     } else {
+        PyErr_SetString(PyExc_TypeError, "Attempt to convert a non-basic "
+                                         "type to ElementPtr");
         throw InternalError();
     }
 }
@@ -239,8 +246,8 @@ logConfigUpdate(PyObject*, PyObject* args) {
         PyErr_SetString(PyExc_TypeError, "argument 2 of log_config_update "
                                          "is not a correct module specification");
     } catch (const InternalError& ie) {
-        PyErr_SetString(PyExc_TypeError, "argument passed to log_config_update "
-                                         "is not a (compound) basic type");
+        // Python exception already set, so no action needed (as long
+        // as we do in fact return NULL right after this block)
     } catch (const std::exception& e) {
         PyErr_SetString(PyExc_RuntimeError, e.what());
     } catch (...) {