Bug 125

Summary:	Ipv4::GetIfIndexByAddress is virtual and has an argument with a default value
Product:	ns-3	Reporter:	Mathieu Lacage <mathieu.lacage>
Component:	network	Assignee:	ns-bugs <ns-bugs>
Status:	RESOLVED FIXED
Severity:	normal
Priority:	P3
Version:	pre-release
Hardware:	All
OS:	All

Description Mathieu Lacage 2008-01-07 05:12:21 UTC

This is really evil: the default value for the argument is not part of the signature so, a subclass can specify a different default value or no default value at all which means that the behavior of the call to GetIfIndexByAddress is going to depend on the type of the pointer used to make the call. 

Virtual methods should _never_ use arguments with default values.

Comment 1 Tom Henderson 2008-01-07 11:32:11 UTC

I was assuming that subclasses would not be inclined to change this default value.  However, there is really no strong need for the mask parameter anyway, if we assume that this function strictly does what it says in the function name.

I'll post a patch later.

Comment 2 Tom Henderson 2008-01-08 10:07:09 UTC

Hmm.. how about just making this non-virtual?

diff -r 4ba90810ae30 src/node/ipv4.h
--- a/src/node/ipv4.h   Tue Jan 08 14:06:49 2008 +0000
+++ b/src/node/ipv4.h   Tue Jan 08 07:04:17 2008 -0800
@@ -451,13 +451,15 @@ public:
 
   /**
    * \brief Convenience function to return the ifIndex corresponding
-   * to the Ipv4Address provided
+   * to the Ipv4Address provided.  If an Ipv4Mask is provided also,
+   * then the address is masked, which allows one to determine the 
+   * interface corresponding to a network prefix.
    *
    * \param addr Ipv4Address
    * \param mask corresponding Ipv4Mask
    * \returns ifIndex corresponding to a/amask
    */
-  virtual uint32_t GetIfIndexByAddress (Ipv4Address addr, 
+  uint32_t GetIfIndexByAddress (Ipv4Address addr, 
     Ipv4Mask mask = Ipv4Mask("255.255.255.255"));
 };

Comment 3 Mathieu Lacage 2008-01-08 14:05:01 UTC

(In reply to comment #2)
> Hmm.. how about just making this non-virtual?

+1

I suspect that the final patch will include the associated change to ipv4.cc and ipv4-impl.h and ipv4-impl.cc

Mathieu

Comment 4 Tom Henderson 2008-01-10 01:19:09 UTC

fixed by changeset 2214 (no need to touch other functions)