Testmeister Tom found a bug in Names::Add for his shortcut semantics
authorCraig Dowell <craigdo@ee.washington.edu>
Fri, 30 Jan 2009 10:30:07 -0800
changeset 4151 682a4cae7a5f
parent 4150 682eb88ed29e
child 4152 99e350f3f7b4
Testmeister Tom found a bug in Names::Add for his shortcut semantics
examples/names.cc
src/core/names.cc
--- a/examples/names.cc	Tue Jan 27 12:39:43 2009 -0800
+++ b/examples/names.cc	Fri Jan 30 10:30:07 2009 -0800
@@ -52,11 +52,17 @@
   //
   // We're going to use the zeroth node in the container as the client, and
   // the first node as the server.  Add some "human readable" names for these
-  // nodes.  The first parameter specifies the root of the "/Names" name space
-  // as the destination, so these will go into the name system as "/Names/client"
-  // and "/Names/server".  
+  // nodes.  The names below will go into the name system as "/Names/client"
+  // and "/Names/server", but note that the Add function assumes that if you
+  // omit the leading "/Names/" the remaining string is assumed to be rooted
+  // in the "/Names" namespace. The following calls,
   //
-  Names::Add ("/Names/client", n.Get (0));
+  //  Names::Add ("client", n.Get (0));
+  //  Names::Add ("/Names/client", n.Get (0));
+  //
+  // will produce identical results.
+  //
+  Names::Add ("client", n.Get (0));
   Names::Add ("/Names/server", n.Get (1));
 
   InternetStackHelper internet;
@@ -71,13 +77,12 @@
   //
   // Add some human readable names for the devices we'll be interested in.
   // We add the names to the name space "under" the nodes we created above.
-  // This has the effect of making "/Names/client/eth0" and "/Names/server/eth0"
-  // Note that the first part of the path must reference a previously named object,
-  // and we have, in fact, already named objects "/Names/client" and
-  // "/Names/server"
+  // This has the effect of making "/Names/client/eth0" and "/Names/server/eth0".
+  // In this case, we again omit the "/Names/" prefix on one call to illustrate
+  // the shortcut.
   //
   Names::Add ("/Names/client/eth0", d.Get (0));
-  Names::Add ("/Names/server/eth0", d.Get (1));
+  Names::Add ("server/eth0", d.Get (1));
 
   //
   // You can use the object names that you've assigned in calls to the Config
@@ -127,7 +132,9 @@
 
   //
   // Use the Config system to connect a trace source using the object name
-  // service to specify the path.
+  // service to specify the path.  Note that in this case, the "/Names"
+  // prefix is always required since the _Config_ system always expects to 
+  // see a fully qualified path name 
   //
   Config::Connect ("/Names/client/eth0/Rx", MakeCallback (&RxEvent));
 
--- a/src/core/names.cc	Tue Jan 27 12:39:43 2009 -0800
+++ b/src/core/names.cc	Fri Jan 30 10:30:07 2009 -0800
@@ -84,8 +84,8 @@
   ~NamesPriv ();
 
   bool Add (std::string name, Ptr<Object> obj);
+  bool Add (std::string context, std::string name, Ptr<Object> object);
   bool Add (Ptr<Object> context, std::string name, Ptr<Object> object);
-  bool Add (std::string context, std::string name, Ptr<Object> object);
   std::string FindShortName (Ptr<Object> object);
   std::string FindFullName (Ptr<Object> object);
   Ptr<Object> FindObjectFromFullName (std::string name);
@@ -167,11 +167,14 @@
   NS_LOG_FUNCTION (name << object);
   //
   // This is the simple, easy to use version of Add, so we want it to be flexible.
+  // We don't want to force a user to always type the fully qualified namespace 
+  // name, so we allow the namespace name to be omitted.  For example, calling
+  // Add ("Client/ath0", obj) should result in exactly the same behavior as
+  // Add ("/Names/Client/ath0", obj).  Calling Add ("Client", obj) should have
+  // the same effect as Add ("Names/Client", obj)
   //
-  // If we are provided a name that doesn't begin with "/Names", we assume 
-  // that the caller has given us a shortname that she wants added to the root
-  // namespace.  This results in a call to the "real" Add with context set to 
-  // zero, indicating what we want to do.
+  // The first thing to do, then, is to "canonicalize" the input string to always
+  // be a fully qualified name.
   //
   // If we are given a name that begins with "/Names/" we assume that this is a
   // fullname to the object we want to create.  We split the fullname into a 
@@ -179,40 +182,45 @@
   //
   std::string namespaceName = "/Names";
   std::string::size_type offset = name.find (namespaceName);
-  if (offset == 0)
+  if (offset != 0)
     {
       //
-      // This must be a fully qualified longname.  All fully qualified names begin
-      // with "/Names".  We have to split off the final segment which will become
-      // the shortname of the object.
-      //
-      std::string::size_type i = name.rfind ("/");
-      NS_ASSERT_MSG (i != std::string::npos, "NamesPriv::Add(): Internal error.  Can't find '/' in name");
-
-      //
-      // The slash we found cannot be the slash at the start of the namespaceName.
-      // This would indicate there is no shortname in the path at all.
-      //
-      NS_ASSERT_MSG (i != 0, "NamesPriv::Add(): Can't find a shortname in the name string");
-
+      // This must be a name that has the "/Names" namespace prefix omitted.  
+      // Do some reasonableness checking on the rest of the name.
       //
-      // We now know where the context string starts and ends, and where the
-      // shortname starts and ends.  All we have to do is to call our available
-      // function for creating addubg a shortname under a context string.
-      //
-      return Add (name.substr (0, i), name.substr (i + 1), object);
+      offset = name.find ("/");
+      if (offset == 0)
+        {
+          NS_ASSERT_MSG (false, "NamesPriv::Add(): Name begins with '/' but not \"/Names\"");
+          return false;
+        }
+
+      name = "/Names/" + name;
     }
-  else
-    {
-      //
-      // This must be a shortname.  Shortnames can't have ANY '/' characters in
-      // them since they are interpreted as a final segment of a fullname.  A 
-      // shortname in this context means creating a name in the root namespace.
-      // We indicate this by passing a zero context to the "real" add.
-      //
-      NS_ASSERT_MSG (offset == std::string::npos, "NamesPriv::Add(): Unexpected '/' in shortname");
-      return Add (Ptr<Object> (0, false), name, object);
-    }
+  
+  //
+  // There must now be a fully qualified longname in the string.  All fully 
+  // qualified names begin with "/Names".  We have to split off the final 
+  // segment which will become the shortname of the object.  A '/' that
+  // separates the context from the final segment had better be there since
+  // we just made sure that at least the namespace name was there.
+  //
+  std::string::size_type i = name.rfind ("/");
+  NS_ASSERT_MSG (i != std::string::npos, "NamesPriv::Add(): Internal error.  Can't find '/' in name");
+
+  //
+  // The slash we found cannot be the slash at the start of the namespaceName.
+  // This would indicate there is no shortname in the path at all.  It can be
+  // any other index.
+  //
+  NS_ASSERT_MSG (i != 0, "NamesPriv::Add(): Can't find a shortname in the name string");
+
+  //
+  // We now know where the context string starts and ends, and where the
+  // shortname starts and ends.  All we have to do is to call our available
+  // function for creating addubg a shortname under a context string.
+  //
+  return Add (name.substr (0, i), name.substr (i + 1), object);
 }
 
 bool
@@ -683,6 +691,65 @@
   NS_TEST_ASSERT_EQUAL (foundObject, serverEth0);
 
   //
+  // We should be able to add objects while including the root of the namespace
+  // in the name.
+  //
+  Ptr<TestObject> router1 = CreateObject<TestObject> ();
+  result = Names::Add ("/Names/Router1", router1);
+  NS_TEST_ASSERT_EQUAL (result, true);
+
+  //
+  // We should be able to add objects while not including the root of the namespace
+  // in the name.
+  //
+  Ptr<TestObject> router2 = CreateObject<TestObject> ();
+  result = Names::Add ("Router2", router2);
+  NS_TEST_ASSERT_EQUAL (result, true);
+
+  //
+  // We should be able to add sub-objects while including the root of the namespace
+  // in the name.
+  //
+  Ptr<TestObject> router1Eth0 = CreateObject<TestObject> ();
+  result = Names::Add ("/Names/Router1/eth0", router1Eth0);
+  NS_TEST_ASSERT_EQUAL (result, true);
+
+  //
+  // We should be able to add sub-objects while not including the root of the namespace
+  // in the name.
+  //
+  Ptr<TestObject> router2Eth0 = CreateObject<TestObject> ();
+  result = Names::Add ("Router2/eth0", router2Eth0);
+  NS_TEST_ASSERT_EQUAL (result, true);
+
+  //
+  // We should be able to find these objects in the same two ways
+  //
+  foundObject = Names::Find<TestObject> ("/Names/Router1");
+  NS_TEST_ASSERT_EQUAL (foundObject, router1);
+
+  foundObject = Names::Find<TestObject> ("Router1");
+  NS_TEST_ASSERT_EQUAL (foundObject, router1);
+
+  foundObject = Names::Find<TestObject> ("/Names/Router2");
+  NS_TEST_ASSERT_EQUAL (foundObject, router2);
+
+  foundObject = Names::Find<TestObject> ("Router2");
+  NS_TEST_ASSERT_EQUAL (foundObject, router2);
+
+  foundObject = Names::Find<TestObject> ("/Names/Router1/eth0");
+  NS_TEST_ASSERT_EQUAL (foundObject, router1Eth0);
+
+  foundObject = Names::Find<TestObject> ("Router1/eth0");
+  NS_TEST_ASSERT_EQUAL (foundObject, router1Eth0);
+
+  foundObject = Names::Find<TestObject> ("/Names/Router2/eth0");
+  NS_TEST_ASSERT_EQUAL (foundObject, router2Eth0);
+
+  foundObject = Names::Find<TestObject> ("Router2/eth0");
+  NS_TEST_ASSERT_EQUAL (foundObject, router2Eth0);
+
+  //
   // We also have some syntactically sugary methods, so make sure they do what
   // they should as well.
   //