update comment, fix typo, etc.

6 files changed, 31 insertions, 15 deletions

diff --git a/src/cnode/cnode-algorithm.hpp b/src/cnode/cnode-algorithm.hpp
index 509f875..d17204f 100644
--- a/src/cnode/cnode-algorithm.hpp
+++ b/src/cnode/cnode-algorithm.hpp
@@ -67,6 +67,11 @@ void showConfig(const std::string& cfg1, const std::string& cfg2,
                 bool hide_secret = false, bool context_diff = false,
                 bool show_cmds = false, bool ignore_edit = false);
 
+/* these functions provide the functionality necessary for the "config
+ * file" shell API. basically the API uses the "cparse" interface to
+ * parse a config file into a CfgNode tree structure, and then these
+ * functions can be used to access the nodes in the tree.
+ */
 CfgNode *findCfgNode(CfgNode *root, const cstore::Cpath& path,
                      bool& is_value);
 CfgNode *findCfgNode(CfgNode *root, const cstore::Cpath& path);
diff --git a/src/cnode/cnode.cpp b/src/cnode/cnode.cpp
index 279ee9a..70e14b8 100644
--- a/src/cnode/cnode.cpp
+++ b/src/cnode/cnode.cpp
@@ -62,8 +62,19 @@ CfgNode::CfgNode(Cpath& path_comps, char *name, char *val, char *comment,
       _is_value = (getTmpl()->isValue() && !_is_leaf);
       _is_multi = getTmpl()->isMulti();
 
-      /* XXX given the current definition of "default", the concept of
-       * "default" doesn't really apply to config files.
+      /* XXX given the current definition of "default" (i.e., the
+       * "post-bug 1219" definition), the concept of "default" doesn't
+       * really apply to config files. however, if in the future we
+       * do go back to the original, simpler definition of "default"
+       * (which IMO is the right thing to do), the "default handling"
+       * here and elsewhere in the backend library will need to be
+       * revamped.
+       *
+       * in fact, in that case pretty much the only place that need to
+       * worry about "default" is in the "output" (i.e., "show")
+       * processing, and even there the only thing that needs to be
+       * done is to compare the current value with the "default value"
+       * in the template.
        */
       _is_default = false;
       _is_deactivated = deact;
diff --git a/src/commit/commit-algorithm.cpp b/src/commit/commit-algorithm.cpp
index 580c9dc..d2612a7 100644
--- a/src/commit/commit-algorithm.cpp
+++ b/src/commit/commit-algorithm.cpp
@@ -117,7 +117,10 @@ _create_commit_cfg_node(CfgNode& cn, const Cpath& p, CommitState s)
   return node;
 }
 
-// "changed" multi-value leaf nodes
+/* "changed" multi-value leaf nodes. note that "changed" state applies to
+ * the "node" itself. the actual states of the node values can only be
+ * added, deleted, or unchanged.
+ */
 static CfgNode *
 _create_commit_cfg_node(const CfgNode& cn, const Cpath& p,
                         const vector<string>& values,
@@ -130,7 +133,7 @@ _create_commit_cfg_node(const CfgNode& cn, const Cpath& p,
   return node;
 }
 
-// "changed" single-value leaf nodes
+// "changed" single-value leaf nodes (this does apply to the value)
 static CfgNode *
 _create_commit_cfg_node(const CfgNode& cn, const Cpath& p, const string& val1,
                         const string& val2, bool def1, bool def2)
@@ -219,7 +222,7 @@ _trv_be_node(N *node)
 template<> bool
 _trv_be_node<CfgNode>(CfgNode *node)
 {
-  return node->isTagNode();
+  return node->isBeginEndNode();
 }
 
 template<class N> static void
diff --git a/src/commit/commit-algorithm.hpp b/src/commit/commit-algorithm.hpp
index fca04cb..0c2980f 100644
--- a/src/commit/commit-algorithm.hpp
+++ b/src/commit/commit-algorithm.hpp
@@ -53,7 +53,7 @@ enum CommitTreeTraversalOrder {
 enum CommitHook {
   PRE_COMMIT,
   POST_COMMIT,
-  LAST
+  LAST // not a valid hook
 };
 
 class CommitData {
diff --git a/src/cstore/cstore-varref.cpp b/src/cstore/cstore-varref.cpp
index 46dba23..f4a6918 100644
--- a/src/cstore/cstore-varref.cpp
+++ b/src/cstore/cstore-varref.cpp
@@ -296,9 +296,6 @@ Cstore::VarRef::getValue(string& value, vtw_type_e& def_type)
 bool
 Cstore::VarRef::getSetPath(Cpath& path_comps)
 {
-  /* XXX this function is currently unused and untested. see setVarRef()
-   *     in Cstore for more information.
-   */
   if (_paths.size() != 1) {
     // for set_var_ref operation, there can be only one path.
     return false;
diff --git a/src/cstore/cstore.cpp b/src/cstore/cstore.cpp
index 6e205ef..ce937c9 100644
--- a/src/cstore/cstore.cpp
+++ b/src/cstore/cstore.cpp
@@ -1707,12 +1707,12 @@ Cstore::setVarRef(const char *ref_str, const char *value, bool to_active)
    *     "syntax:", it will not be performed during "set" (but will be
    *     during commit).
    *
-   *     since commit has not been converted to use the new library, it
-   *     does not use this function. instead, it uses the "cli_val_engine"
-   *     implementation (where filesystem paths are deeply embedded, which
-   *     makes it difficult to abstract low-level filesystem operations
-   *     from high-level functions). as a result, this function is unused
-   *     and untested at the moment. must revisit when converting commit.
+   * XXX also the behavior here follows the original implementation and
+   *     has these limitations:
+   *     * it does not check if the type of the specified value is
+   *       correct, e.g., it may write a txt value to a u32 node if
+   *       that's what the template specifies.
+   *     * it only supports only single-value leaf nodes.
    */
   auto_ptr<SavePaths> save(create_save_paths());
   VarRef vref(this, ref_str, to_active);