r75452 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r75451‎ | r75452 | r75453 >
Date:18:41, 26 October 2010
Author:nimishg
Status:deferred (Comments)
Tags:
Comment:
added multicast support
Modified paths:
  • /trunk/udplog/srclib/Socket.h (modified) (history)
  • /trunk/udplog/udp2log/udp2log.cpp (modified) (history)

Diff [purge]

Index: trunk/udplog/udp2log/udp2log.cpp
@@ -22,6 +22,7 @@
2323 std::string logFileName("/var/log/udp2log/udp2log.log");
2424 std::string pidFileName("/var/run/udp2log.pid");
2525 std::string daemonUserName("udp2log");
 26+std::string multicastAddr("0");
2627
2728 Udp2LogConfig config;
2829
@@ -112,10 +113,11 @@
113114 using namespace boost::program_options;
114115 unsigned int port = 8420;
115116 bool daemon = false;
116 -
 117+
117118 // Process command line
118119 options_description optDesc;
119120 optDesc.add_options()
 121+ ("multicast", value<string>(&multicastAddr)->default_value(multicastAddr), "multicast address to listen to")
120122 ("help", "Show help message.")
121123 ("port,p", value<unsigned int>(&port)->default_value(port), "UDP port.")
122124 ("config-file,f", value<string>(&configFileName)->default_value(configFileName),
@@ -182,13 +184,18 @@
183185 // Open the receiving socket
184186 IPAddress any(INADDR_ANY);
185187 SocketAddress saddr(any, (unsigned short int)port);
186 - UDPSocket socket;
187 - if (!socket) {
188 - cerr << "Unable to open socket\n";
189 - return 1;
190 - }
191 - socket.Bind(saddr);
192 -
 188+ Socket socket;
 189+ if(strcmp("0", multicastAddr.c_str())){ //if multicast
 190+ socket = *(new MulticastSocket(any, port, multicastAddr.c_str()));
 191+ }
 192+ else{
 193+ socket = *(new UDPSocket());
 194+ if (!socket) {
 195+ cerr << "Unable to open socket\n";
 196+ return 1;
 197+ }
 198+ socket.Bind(saddr);
 199+ }
193200 // Process received packets
194201 boost::shared_ptr<SocketAddress> address;
195202 const size_t bufSize = 65536;
Index: trunk/udplog/srclib/Socket.h
@@ -15,6 +15,7 @@
1616 class Socket
1717 {
1818 public:
 19+ Socket(){}
1920 Socket(int domain, int type, int protocol)
2021 : fd(-1)
2122 {
@@ -153,4 +154,29 @@
154155 }
155156 };
156157
 158+
 159+class MulticastSocket : public Socket
 160+{
 161+public:
 162+ MulticastSocket(int domain = PF_INET)
 163+ : Socket(domain, SOCK_DGRAM, 0) {}
 164+
 165+ MulticastSocket(IPAddress & addr, unsigned int port, const char* multicastAddr)
 166+ : Socket(addr.GetDomain(), SOCK_DGRAM, 0)
 167+ {
 168+ struct ip_mreq imreq;
 169+ bzero(&imreq, sizeof(struct ip_mreq));
 170+ boost::shared_ptr<SocketAddress> saddr = addr.NewSocketAddress(port);
 171+ if (Connect(*saddr)) {
 172+ good = false;
 173+ }
 174+ imreq.imr_multiaddr.s_addr = inet_addr(multicastAddr);
 175+ imreq.imr_interface.s_addr = INADDR_ANY;
 176+
 177+ // JOIN multicast group on default interface
 178+ setsockopt(fd, IPPROTO_IP, IP_ADD_MEMBERSHIP,
 179+ (const void *)&imreq, sizeof(struct ip_mreq));
 180+
 181+ }
 182+};
157183 #endif

Follow-up revisions

RevisionCommit summaryAuthorDate
r100788* Moved the multicast join function out of the generic Socket class and into ...tstarling04:49, 26 October 2011

Comments

#Comment by Reedy (talk | contribs)   18:43, 26 October 2010

A few bits and pieces look out of alignment...

#Comment by Tim Starling (talk | contribs)   02:46, 26 October 2011
Socket socket;
...
socket = *(new MulticastSocket(any, port, multicastAddr.c_str()));

This creates a new MulticastSocket on the heap, copies it member-by-member into a Socket object on the stack, and then loses the pointer to the MulticastSocket, leaking memory. The resulting object is not a MulticastSocket, calling a method on it will result in a call to Socket, not MulticastSocket.

This was partially fixed by r95725, I just thought I'd mention here for future reference that this is not how polymorphism in C++ works.

Status & tagging log