Add metric in Ipv4StaticRouting::PrintRoutingTable

Add metric in Ipv4StaticRouting::PrintRoutingTable

[Mathieu Lacage, 2012-03-09 19:54:42]

http://codereview.appspot.com/5784068/diff/1/src/internet/model/ipv4-static-r...
File src/internet/model/ipv4-static-routing.h (left):

http://codereview.appspot.com/5784068/diff/1/src/internet/model/ipv4-static-r...
src/internet/model/ipv4-static-routing.h:376: 
why ? This is a no-no for me.

[Tommaso Pecorella, 2012-03-10 00:18:23]

Th patch seems fine except for the m_ipv4 change from private to protected.

T.

http://codereview.appspot.com/5784068/diff/1/src/internet/model/ipv4-static-r...
File src/internet/model/ipv4-static-routing.h (right):

http://codereview.appspot.com/5784068/diff/1/src/internet/model/ipv4-static-r...
src/internet/model/ipv4-static-routing.h:376: Ptr<Ipv4> m_ipv4;
I have no idea about why m_ipv4 should be protected, the patch does not seems to
need it, so please put it back where it belongs.

[Frederic U, 2012-03-12 10:36:38]

On 2012/03/10 00:18:23, Tommaso Pecorella wrote:
> Th patch seems fine except for the m_ipv4 change from private to protected.
> 
> T.
> 
>
http://codereview.appspot.com/5784068/diff/1/src/internet/model/ipv4-static-r...
> File src/internet/model/ipv4-static-routing.h (right):
> 
>
http://codereview.appspot.com/5784068/diff/1/src/internet/model/ipv4-static-r...
> src/internet/model/ipv4-static-routing.h:376: Ptr<Ipv4> m_ipv4;
> I have no idea about why m_ipv4 should be protected, the patch does not seems
to
> need it, so please put it back where it belongs.

Done.

[Frederic U, 2012-03-14 10:32:08]

No news, so I can commit ?

Frederic.

On 03/12/2012 11:36 AM, frederic.urbani@inria.fr wrote:
> Reviewers: Mathieu Lacage, Tommaso Pecorella,
>
> Message:
> On 2012/03/10 00:18:23, Tommaso Pecorella wrote:
>> Th patch seems fine except for the m_ipv4 change from private to
> protected.
>
>> T.
>
>
>
http://codereview.appspot.com/5784068/diff/1/src/internet/model/ipv4-static-r...

>
>> File src/internet/model/ipv4-static-routing.h (right):
>
>
>
http://codereview.appspot.com/5784068/diff/1/src/internet/model/ipv4-static-r...

>
>> src/internet/model/ipv4-static-routing.h:376: Ptr<Ipv4> m_ipv4;
>> I have no idea about why m_ipv4 should be protected, the patch does
> not seems to
>> need it, so please put it back where it belongs.
>
> Done.
>
> Description:
> Add metric in Ipv4StaticRouting::PrintRoutingTable
>
> Please review this at http://codereview.appspot.com/5784068/
>
> Affected files:
>   M src/internet/model/ipv4-static-routing.cc
>   M src/internet/model/ipv4-static-routing.h
>
>
> Index: src/internet/model/ipv4-static-routing.cc
> ===================================================================
> --- a/src/internet/model/ipv4-static-routing.cc
> +++ b/src/internet/model/ipv4-static-routing.cc
> @@ -413,11 +413,11 @@
>  }
>
>  uint32_t
> -Ipv4StaticRouting::GetMetric (uint32_t index)
> +Ipv4StaticRouting::GetMetric (uint32_t index) const
>  {
>    NS_LOG_FUNCTION (this << index);
>    uint32_t tmp = 0;
> -  for (NetworkRoutesI j = m_networkRoutes.begin ();
> +  for (NetworkRoutesCI j = m_networkRoutes.begin ();
>         j != m_networkRoutes.end ();
>         j++)
>      {
> @@ -700,7 +700,6 @@
>          }
>      }
>  }
> -
>  // Formatted like output of "route -n" command
>  void
>  Ipv4StaticRouting::PrintRoutingTable (Ptr<OutputStreamWrapper> 
> stream) const
> @@ -713,11 +712,11 @@
>          {
>            std::ostringstream dest, gw, mask, flags;
>            Ipv4RoutingTableEntry route = GetRoute (j);
> -          dest << route.GetDest ();
> +          dest << route.GetDest ();
>            *os << std::setiosflags (std::ios::left) << std::setw (16) 
> << dest.str ();
> -          gw << route.GetGateway ();
> +          gw << route.GetGateway ();
>            *os << std::setiosflags (std::ios::left) << std::setw (16) 
> << gw.str ();
> -          mask << route.GetDestNetworkMask ();
> +          mask << route.GetDestNetworkMask ();
>            *os << std::setiosflags (std::ios::left) << std::setw (16) 
> << mask.str ();
>            flags << "U";
>            if (route.IsHost ())
> @@ -729,8 +728,7 @@
>                flags << "GS";
>              }
>            *os << std::setiosflags (std::ios::left) << std::setw (6) 
> << flags.str ();
> -          // Metric not implemented
> -          *os << "-" << "      ";
> +          *os << std::setiosflags (std::ios::left) << std::setw (7) 
> << GetMetric (j);
>            // Ref ct not implemented
>            *os << "-" << "      ";
>            // Use not implemented
> @@ -747,7 +745,6 @@
>          }
>      }
>  }
> -
>  Ipv4Address
>  Ipv4StaticRouting::SourceAddressSelection (uint32_t interfaceIdx, 
> Ipv4Address dest)
>  {
> Index: src/internet/model/ipv4-static-routing.h
> ===================================================================
> --- a/src/internet/model/ipv4-static-routing.h
> +++ b/src/internet/model/ipv4-static-routing.h
> @@ -213,7 +213,7 @@
>   * \return If route is set, the metric is returned. If not, an 
> infinity metric (0xffffffff) is returned
>   *
>   */
> -  uint32_t GetMetric (uint32_t index);
> +  uint32_t GetMetric (uint32_t index) const;
>
>  /**
>   * \brief Remove a route from the static unicast routing table.
>
>

[tomh_tomh.org, 2012-03-14 15:46:02]

On 03/14/2012 03:32 AM, Frederic Urbani wrote:
> No news, so I can commit ?
>
> Frederic.
>

OK by me.

[Tommaso Pecorella, 2012-03-14 18:12:14]

On 2012/03/14 10:32:08, Frederic U wrote:
> No news, so I can commit ?
> 
> Frederic.

+1, go ahead.

Tommaso