r95725 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r95724‎ | r95725 | r95726 >
Date:23:21, 29 August 2011
Author:asher
Status:resolved (Comments)
Tags:
Comment:
rewrite udp2log multicast listen support
Modified paths:
  • /trunk/udplog/debian/changelog (modified) (history)
  • /trunk/udplog/srclib/Socket.h (modified) (history)
  • /trunk/udplog/udp2log/udp2log.cpp (modified) (history)

Diff [purge]

Index: trunk/udplog/debian/changelog
@@ -1,3 +1,9 @@
 2+udplog (1.7-1) lucid; urgency=low
 3+
 4+ * Rewrite multicast listen support
 5+
 6+ -- Asher Feldman <afledman@wikimedia.org> Mon, 29 Aug 2011 23:17:35 +0000
 7+
28 udplog (1.6-1) lucid; urgency=low
39
410 * Fixed account creation in postinst: create system account.
Index: trunk/udplog/udp2log/udp2log.cpp
@@ -185,17 +185,15 @@
186186 IPAddress any(INADDR_ANY);
187187 SocketAddress saddr(any, (unsigned short int)port);
188188 Socket socket;
 189+ socket = *(new UDPSocket());
 190+ if (!socket) {
 191+ cerr << "Unable to open socket\n";
 192+ return 1;
 193+ }
 194+ socket.Bind(saddr);
189195 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 - }
 196+ socket.Mcast(multicastAddr.c_str());
 197+ }
200198 // Process received packets
201199 boost::shared_ptr<SocketAddress> address;
202200 const size_t bufSize = 65536;
Index: trunk/udplog/srclib/Socket.h
@@ -62,6 +62,22 @@
6363 }
6464 }
6565
 66+ int Mcast(const char* multicastAddr) {
 67+ IPAddress any(INADDR_ANY);
 68+ struct ip_mreq imreq;
 69+
 70+ bzero(&imreq, sizeof(struct ip_mreq));
 71+ imreq.imr_multiaddr.s_addr = inet_addr(multicastAddr);
 72+ imreq.imr_interface.s_addr = INADDR_ANY;
 73+ if (setsockopt(fd, IPPROTO_IP, IP_ADD_MEMBERSHIP, (const void *)&imreq,
 74+ sizeof(struct ip_mreq))) {
 75+ RaiseError("Socket::Mcast");
 76+ return errno;
 77+ } else {
 78+ return 0;
 79+ }
 80+ }
 81+
6682 boost::shared_ptr<SocketAddress> GetPeerAddress() {
6783 if (connect(fd, SocketAddress::GetBuffer(), SocketAddress::GetBufferLength()) < 0) {
6884 RaiseError("Socket::GetPeerAddress");
@@ -154,29 +170,4 @@
155171 }
156172 };
157173
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 -};
183174 #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 Tim Starling (talk | contribs)   02:54, 26 October 2011
Socket socket;
socket = *(new UDPSocket());

This is less broken than the code that came before (I wrote a comment about it on r75452), but it still leaks memory and incorrectly does a copies the contents of a derived class to its parent class, a parent class which has a comment attached to it explicitly saying that it shouldn't be instantiated.

I guess I will have to fix this and make Socket properly uncopyable, to avoid similar mistakes in the future.

Status & tagging log