r100788 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r100787‎ \| r100788 \| r100789 >
Date:	04:49, 26 October 2011
Author:	tstarling
Status:	ok
Tags:
Comment:	* Moved the multicast join function out of the generic Socket class and into UDPSocket where it belongs, renamed it to JoinMulticast. Have it accept an IPAddress instead of char* per convention. * Rewrote JoinMulticast, implemented IPv6 multicast group join support. * Fixed the non-functional char* constructor of IPAddress. * Added a constructor from std::string to IPAddress, for convenience. * Added some IPAddress wrappers for INADDR_ANY and in6addr_any. * Sprinkled in some const modifiers. * Enforced the non-copyable nature of Socket with a private constructor. * Enforced the fact that Socket is a pure base class and should not be instantiated, by making all constructors private or protected. * In udp2log, fixed the ridiculous attempt at polymorphic object construction introduced in r75452, mimicked in r95725. * Moved -lboost_program_options to the end of the object list, the linker now seems to want this.
Modified paths:	/trunk/udplog/Makefile (modified) (history) /trunk/udplog/srclib/IPAddress.cpp (modified) (history) /trunk/udplog/srclib/IPAddress.h (modified) (history) /trunk/udplog/srclib/Socket.cpp (modified) (history) /trunk/udplog/srclib/Socket.h (modified) (history) /trunk/udplog/srclib/SocketAddress.h (modified) (history) /trunk/udplog/udp2log/udp2log.cpp (modified) (history)

Diff [purge]

Index: trunk/udplog/udp2log/udp2log.cpp
—	—	@@ -182,17 +182,22 @@
183	183	signal(SIGPIPE, SIG_IGN);
184	184
185	185	// Open the receiving socket
186		~~- IPAddress any(INADDR_ANY);~~
187		~~- SocketAddress saddr(any, (unsigned short int)port);~~
188		~~- Socket socket;~~
189		~~- socket = *(new UDPSocket());~~
	186	+ SocketAddress saddr(IPAddress::any4, (unsigned short int)port);
	187	+ UDPSocket socket;
190	188	if (!socket) {
191	189	cerr << "Unable to open socket\n";
192	190	return 1;
193	191	}
194	192	socket.Bind(saddr);
195		~~- if(strcmp("0", multicastAddr.c_str())){ //if multicast~~
196		~~- socket.Mcast(multicastAddr.c_str());~~
	193	+
	194	+ // Join a multicast group if requested
	195	+ if (multicastAddr != "0") {
	196	+ try {
	197	+ socket.JoinMulticast(multicastAddr);
	198	+ } catch (runtime_error & e) {
	199	+ cerr << "Error joining multicast group: " << e.what() << endl;
	200	+ return 1;
	201	+ }
197	202	}
198	203	// Process received packets
199	204	boost::shared_ptr<SocketAddress> address;
Index: trunk/udplog/Makefile
—	—	@@ -17,7 +17,7 @@
18	18	g++ $(CFLAGS) $(HOST_OBJS) -o host
19	19
20	20	log2udp: $(LOG2UDP_OBJS)
21		~~- g++ $(CFLAGS) -lboost_program_options $(LOG2UDP_OBJS) -o log2udp~~
	21	+ g++ $(CFLAGS) $(LOG2UDP_OBJS) -lboost_program_options -o log2udp
22	22
23	23	udprecv: $(UDPRECV_OBJS)
24	24	g++ $(CFLAGS) -Wall $(UDPRECV_OBJS) -o udprecv
—	—	@@ -29,7 +29,7 @@
30	30	g++ $(CFLAGS) -o packet-loss srcmisc/packet-loss.cpp
31	31
32	32	udp2log/udp2log: $(UDP2LOG_OBJS)
33		~~- g++ $(CFLAGS) -o udp2log/udp2log -lboost_program_options $(UDP2LOG_OBJS)~~
	33	+ g++ $(CFLAGS) -o udp2log/udp2log $(UDP2LOG_OBJS) -lboost_program_options
34	34
35	35	install:
36	36	install log2udp $(DESTDIR)/usr/bin/log2udp
Index: trunk/udplog/srclib/IPAddress.cpp
—	—	@@ -2,6 +2,9 @@
3	3	#include "Exception.h"
4	4	#include "SocketAddress.h"
5	5
	6	+const IPAddress IPAddress::any4(INADDR_ANY);
	7	+const IPAddress IPAddress::any6(&in6addr_any);
	8	+
6	9	std::string & IPAddress::ToString()
7	10	{
8	11	if (!presentation.size()) {
—	—	@@ -19,8 +22,17 @@
20	23	return presentation;
21	24	}
22	25
23		~~-IPAddress::IPAddress(int type, char *presentation)~~
	26	+void IPAddress::InitFromString(const char *presentation)
24	27	{
	28	+ domain = PF_INET;
	29	+ if (strstr(presentation, ":")) {
	30	+ type = AF_INET6;
	31	+ length = sizeof(in6_addr);
	32	+ } else {
	33	+ type = AF_INET;
	34	+ length = sizeof(in_addr);
	35	+ }
	36	+
25	37	int ret = inet_pton(type, presentation, &data);
26	38	if (!ret ) {
27	39	throw std::runtime_error("Invalid IP address");
Index: trunk/udplog/srclib/IPAddress.h
—	—	@@ -5,6 +5,8 @@
6	6	#include <netinet/in.h>
7	7	#include <string>
8	8	#include <boost/shared_ptr.hpp>
	9	+#include <stdexcept>
	10	+#include <memory.h>
9	11
10	12	class SocketAddress;
11	13
—	—	@@ -13,13 +15,13 @@
14	16	IPAddress()
15	17	: type(AF_INET), domain(PF_INET), length(sizeof(in_addr)) {}
16	18
17		~~- IPAddress(struct in_addr * src)~~
	19	+ IPAddress(const struct in_addr * src)
18	20	: type(AF_INET), domain(PF_INET), length(sizeof(in_addr))
19	21	{
20	22	data.v4 = *src;
21	23	}
22	24
23		~~- IPAddress(struct in6_addr * src)~~
	25	+ IPAddress(const struct in6_addr * src)
24	26	: type(AF_INET6), domain(PF_INET6), length(sizeof(in6_addr))
25	27	{
26	28	data.v6 = *src;
—	—	@@ -31,16 +33,34 @@
32	34	data.v4.s_addr = addr;
33	35	}
34	36
35		~~- IPAddress(int type, char * presentation);~~
	37	+ IPAddress(const char * presentation) {
	38	+ InitFromString(presentation);
	39	+ }
36	40
37		~~- void* GetBinaryData() { return &data; }~~
38		~~- size_t GetBinaryLength() { return length; }~~
39		~~- int GetType() { return type; }~~
40		~~- int GetDomain() { return domain; }~~
	41	+ IPAddress(const std::string & s) {
	42	+ if (s.find('\0') != std::string::npos) {
	43	+ throw std::runtime_error("Invalid IP address");
	44	+ }
	45	+ InitFromString(s.c_str());
	46	+ }
	47	+
	48	+ const void* GetBinaryData() const { return &data; }
	49	+ size_t GetBinaryLength() const { return length; }
	50	+
	51	+ void CopyBinaryData(void * dest) const {
	52	+ memcpy(dest, GetBinaryData(), GetBinaryLength());
	53	+ }
	54	+
	55	+ int GetType() const { return type; }
	56	+ int GetDomain() const { return domain; }
41	57	std::string & ToString();
42	58	boost::shared_ptr<SocketAddress> NewSocketAddress(unsigned short int port);
43	59
	60	+ static const IPAddress any4;
	61	+ static const IPAddress any6;
44	62	protected:
	63	+ void InitFromString(const char * presentation);
	64	+
45	65	enum { BUFSIZE = 200 };
46	66
47	67	std::string presentation;
—	—	@@ -49,6 +69,8 @@
50	70	struct in_addr v4;
51	71	} data;
52	72	int type, domain, length;
	73	+
	74	+
53	75	};
54	76
55	77	#endif
Index: trunk/udplog/srclib/SocketAddress.h
—	—	@@ -36,7 +36,7 @@
37	37	data.v6.sin6_family = AF_INET6;
38	38	}
39	39
40		~~- SocketAddress(IPAddress & ip, unsigned short int port)~~
	40	+ SocketAddress(const IPAddress & ip, unsigned short int port)
41	41	{
42	42	switch (ip.GetType()) {
43	43	case AF_INET:
Index: trunk/udplog/srclib/Socket.cpp
—	—	@@ -19,3 +19,35 @@
20	20	}
21	21	}
22	22
	23	+int UDPSocket::JoinMulticast(const IPAddress & addr) {
	24	+ int level, optname;
	25	+ void *optval;
	26	+ socklen_t optlen;
	27	+ struct ip_mreqn mreq4;
	28	+ struct ipv6_mreq mreq6;
	29	+
	30	+ if (addr.GetType() == AF_INET) {
	31	+ addr.CopyBinaryData(&mreq4.imr_multiaddr.s_addr);
	32	+ IPAddress::any4.CopyBinaryData(&mreq4.imr_address);
	33	+ mreq4.imr_ifindex = 0;
	34	+ level = IPPROTO_IP;
	35	+ optname = IP_ADD_MEMBERSHIP;
	36	+ optval = &mreq4;
	37	+ optlen = sizeof(mreq4);
	38	+ } else {
	39	+ addr.CopyBinaryData(&mreq6.ipv6mr_multiaddr);
	40	+ mreq6.ipv6mr_interface = 0;
	41	+ level = IPPROTO_IPV6;
	42	+ optname = IPV6_JOIN_GROUP;
	43	+ optval = &mreq6;
	44	+ optlen = sizeof(mreq6);
	45	+ }
	46	+
	47	+ if (setsockopt(fd, level, optname, optval, optlen)) {
	48	+ RaiseError("UDPSocket::JoinMulticast");
	49	+ return errno;
	50	+ } else {
	51	+ return 0;
	52	+ }
	53	+}
	54	+
Index: trunk/udplog/srclib/Socket.h
—	—	@@ -14,8 +14,14 @@
15	15	// Do not instantiate this directly
16	16	class Socket
17	17	{
18		~~-public:~~
	18	+private:
	19	+ // Not default constructible
19	20	Socket(){}
	21	+
	22	+ // Not copyable
	23	+ Socket(const Socket & s) {}
	24	+
	25	+protected:
20	26	Socket(int domain, int type, int protocol)
21	27	: fd(-1)
22	28	{
—	—	@@ -28,6 +34,8 @@
29	35	}
30	36	}
31	37
	38	+public:
	39	+
32	40	operator bool() {
33	41	return good;
34	42	}
—	—	@@ -62,22 +70,6 @@
63	71	}
64	72	}
65	73
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		-
82	74	boost::shared_ptr<SocketAddress> GetPeerAddress() {
83	75	if (connect(fd, SocketAddress::GetBuffer(), SocketAddress::GetBufferLength()) < 0) {
84	76	RaiseError("Socket::GetPeerAddress");
—	—	@@ -168,6 +160,11 @@
169	161	good = false;
170	162	}
171	163	}
	164	+
	165	+ int JoinMulticast(const IPAddress & addr);
	166	+private:
	167	+ int JoinMulticast4(const IPAddress & addr);
	168	+ int JoinMulticast6(const IPAddress & addr);
172	169	};
173	170
174	171	#endif

Past revisions this follows-up on

Revision	Commit summary	Author	Date
r75452	added multicast support	nimishg	18:41, 26 October 2010
r95725	rewrite udp2log multicast listen support	asher	23:21, 29 August 2011

Status & tagging log

03:52, 1 November 2011 Afeldman (talk | contribs) changed the status of r100788 [removed: new added: ok]