Bug 281

Summary: Socket::CreateSocket (node, "string") does not do what you think
Product: ns-3 Reporter: Mathieu Lacage <mathieu.lacage>
Component: networkAssignee: ns-bugs <ns-bugs>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P3    
Version: pre-release   
Hardware: All   
OS: All   
Attachments: Testcase and fix in one

Description Mathieu Lacage 2008-08-11 17:17:18 UTC
the above code compiles but the type of the second arg of CreateSocket is a TypeId, not an std::string, to a TypeId is implicitely constructed as a temporary variable but this TypeId (char *) constructor does not do a lookup for the requested TypeId. It, instead, attempts to create a new TypeId which, in this case, is likely to trigger the warning:
"Trying to allocate twice the same uid:"
in ../src/core/type-id.cc:115

What are the fixes ? A simple fix might be to make CreateSocket take a std::string instead of a TypeId. I have not verified it would work though.
Comment 1 Rajib Bhattacharjea 2008-08-12 12:42:50 UTC
How about the explicit keyword on the TypeId(char *) constructor to prevent the implicit conversion.  This would force the user to put in the TypeId he actually means instead of letting that constructor do the wrong thing.
Comment 2 Rajib Bhattacharjea 2008-08-13 16:38:04 UTC
Created attachment 228 [details]
Testcase and fix in one

I changed the PacketSink call to Socket::CreateSocket to pass the correct const char* instead of a TypeId (this is the test case).

The fix is the explicit keyword.  As you will see, the compiler doesn't do the implicit conversion from const char* using the offending TypeId constructor any longer, and it won't compile this kind of code instead of subtly failing at runtime:

../src/applications/packet-sink/packet-sink.cc:88: error: no matching function for call to 'ns3::Socket::CreateSocket(ns3::Ptr<ns3::Node>, const char*)'
debug/ns3/socket.h:94: note: candidates are: static ns3::Ptr<ns3::Socket> ns3::Socket::CreateSocket(ns3::Ptr<ns3::Node>, ns3::TypeId)

Mathieu, comments?
Comment 3 Mathieu Lacage 2008-08-14 11:28:09 UTC
(In reply to comment #2)
> Created an attachment (id=228) [details]
> Testcase and fix in one
> 
> I changed the PacketSink call to Socket::CreateSocket to pass the correct const
> char* instead of a TypeId (this is the test case).
> 
> The fix is the explicit keyword.  As you will see, the compiler doesn't do the
> implicit conversion from const char* using the offending TypeId constructor any
> longer, and it won't compile this kind of code instead of subtly failing at
> runtime:
> 
> ../src/applications/packet-sink/packet-sink.cc:88: error: no matching function
> for call to 'ns3::Socket::CreateSocket(ns3::Ptr<ns3::Node>, const char*)'
> debug/ns3/socket.h:94: note: candidates are: static ns3::Ptr<ns3::Socket>
> ns3::Socket::CreateSocket(ns3::Ptr<ns3::Node>, ns3::TypeId)
> 
> Mathieu, comments?

if it works, I am all for it. Please, apply.
Comment 4 Rajib Bhattacharjea 2008-08-14 12:58:28 UTC
Fixed in:

3531:e5c71362e669