[Ns-developers] About real-world application integration

[Mathieu Lacage]

hi,

On Wed, 2009-03-11 at 17:54 +0100, Mathieu Lacage wrote:

> code, and I will do a more thorough review of your code sometime next
> week (I am supposed to be away from email this week :).

So, I did look at your code :) A couple of random comments:

1) you checked in a couple of emacs-generated
files: /scratch/process-zebra.cc~, scratch/process-zebra.py~

2) I see a couple of patches to the ipv6 code: it would be nice if you
could contribute these back to sebastien separately.

3) In src/node/node.cc, what is the use of this extra tag ? Where is it
used ?

+  SocketRecvIfTag tag;
+  tag.SetRecvIf (device->GetIfIndex());
+  packet->AddTag (tag);
+

4) I see a couple of patches to src/process-manager/* which could be
merged in ns-3-simu right now, so, it would be nice if you could try to
split them from the rest to try to minimize the number of differences
between your tree and ns-3-simu. For example, the following looks like a
good idea and I would be happy to take patches to add them to
http://code.nsnam.org/mathieu/ns-3-simu

diff -r 89718962b257 -r 6066282751f1 src/process-manager/alloc.cc
--- a/src/process-manager/alloc.cc      Fri Feb 27 23:57:07 2009 +0900
+++ b/src/process-manager/alloc.cc      Wed Mar 11 15:03:07 2009 +0900
@@ -54,6 +54,7 @@
   : m_defaultMmapSize (1<<15)
 {
   NS_LOG_FUNCTION (this);
+  memset(m_buckets, 0, sizeof(m_buckets));
 }
 KingsleyAlloc::~KingsleyAlloc ()
 {

this too:
diff -r cc696412ba7e -r 9bd77de2052c
src/process-manager/unix-datagram-socket-fd.cc
--- a/src/process-manager/unix-datagram-socket-fd.cc    Fri Feb 27
23:58:01 2009 +0900
+++ b/src/process-manager/unix-datagram-socket-fd.cc    Fri Mar 06
13:25:32 2009 +0900
@@ -181,7 +181,7 @@
   Thread *current = Current ();
   NS_LOG_FUNCTION (this << current);
   NS_ASSERT (current != 0);
-  uint32_t old_ifindex;
+  uint32_t old_ifindex = 0;


as well as:

-  process->alloc = new StupidAlloc ();
-  //  process->alloc = new KingsleyAlloc ();
+  process->alloc = new KingsleyAlloc ();

and, I assume that this made a bit difference for you:

-  std::vector<std::pair<int,Ptr<UnixFd> > > openFiles;
+  __gnu_cxx::hash_map<int, Ptr<UnixFd> > openFiles;

but, oh, look at src/internet-stack/sgi-hashmap.h: would you mind move
it to src/core and use this one instead ?



The following looks a bit gratuituous:

diff -r 89718962b257 -r 6066282751f1
src/process-manager/process-manager.cc
--- a/src/process-manager/process-manager.cc    Fri Feb 27 23:57:07 2009
+0900
+++ b/src/process-manager/process-manager.cc    Wed Mar 11 15:03:07 2009
+0900
@@ -109,12 +109,12 @@
 {
   NS_LOG_FUNCTION (this);
   GarbageCollectDeadProcessesAndThreads ();
-  for (std::vector<struct Process *>::iterator i = m_processes.begin
(); i != m_processes.end (); ++i)
+  for (std::vector<struct Process *>::iterator i = m_processes.begin
(); i != m_processes.end ();)
     {
       struct Process *process = *i;
       DeleteProcess (process);
+      i = m_processes.erase (i);
     }
-  m_processes.clear ();

This would be nice rewritten with the coding style in mind:

+  if(current->process->openFiles.find(fd) !=
current->process->openFiles.end())
+    return fd;

i.e.:
if ()
  {
    // do stuff
  }

again, thanks for your work,
Mathieu