[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH] dhcprelay
From:       Bernhard Fischer <rep.nop () aon ! at>
Date:       2006-11-21 12:03:00
Message-ID: 20061121120300.GA3917 () aon ! at
[Download RAW message or body]

On Tue, Nov 21, 2006 at 12:35:19PM +0100, Bernhard Fischer wrote:
>On Mon, Nov 20, 2006 at 08:44:27PM +0100, Denis Vlasenko wrote:
>>On Monday 20 November 2006 13:23, Bernhard Fischer wrote:
>>> On Fri, Nov 03, 2006 at 12:13:52AM +0100, Denis Vlasenko wrote:
>>> >On Thursday 02 November 2006 12:29, Bernhard Fischer wrote:
>
>>> >> Didn't look at the code, but it sounds useful.
>>> >> vda, care to apply this?
>>> >Sure. Where's the patch?
>>> It was posted earlier in this thread.
>>Applied with some amount of manual fixing. :(
>>Please test current svn...
>>
>>
>>If sendto will for some unfathomable reason return
>>positive res here:
>>
>>        res = sendto(fds[0], p, packet_len, 0, (struct sockaddr*)server_addr,
>>                        sizeof(struct sockaddr_in));
>>        if (res != packet_len) {
>>                bb_perror_msg("pass_on");
>>                return;
>>        }
>>
>>We will get dreaded "Success" error message:
>>
>>dhcpreplay: pass_on: Success
>>
>>Utterly confusing.
>>
>>Maybe check for < 0 return value?
>
>Also:
>listen_socket:
>	- we have these twice. Once static in dnsd.c and the ones in
>	  uchdp/; The latter does always die in it's callers but doesn't
>	  use xsocket..

To outline what i mean, i'm attaching a completely untested patchlet,
fwiw.

$ make CC=gcc-4.3-HEAD bloatcheck
function                                             old     new   delta
dnsd_main                                           1967    1954     -13
udhcp_listen_socket                                  284     263     -21
udhcpd_main                                         1187    1161     -26
udhcpc_main                                         2750    2724     -26
dhcprelay_main                                      1285    1254     -31
raw_socket                                           115      75     -40
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 0/6 up/down: 0/-157)           Total: -157 bytes

Just a thought..
cheers,
>
>networking/udhcp/common.h:
>	These silly #defines should IMHO go away.

["busybox.socketstuff.diff" (text/plain)]

Index: networking/dnsd.c
===================================================================
--- networking/dnsd.c	(revision 16602)
+++ networking/dnsd.c	(working copy)
@@ -199,11 +199,11 @@ static int listen_socket(char *iface_add
 {
 	struct sockaddr_in a;
 	char msg[100];
-	int s;
+	int sck;
 	int yes = 1;
-	s = xsocket(PF_INET, SOCK_DGRAM, 0);
+	sck = xsocket(PF_INET, SOCK_DGRAM, 0);
 #ifdef SO_REUSEADDR
-	if (setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (char *)&yes, sizeof(yes)) < 0)
+	if (setsockopt(sck, SOL_SOCKET, SO_REUSEADDR, (char *)&yes, sizeof(yes)) < 0)
 		bb_perror_msg_and_die("setsockopt() failed");
 #endif
 	memset(&a, 0, sizeof(a));
@@ -211,12 +211,12 @@ static int listen_socket(char *iface_add
 	a.sin_family = AF_INET;
 	if (!inet_aton(iface_addr, &a.sin_addr))
 		bb_perror_msg_and_die("bad iface address");
-	xbind(s, (struct sockaddr *)&a, sizeof(a));
-	xlisten(s, 50);
+	xbind(sck, (struct sockaddr *)&a, sizeof(a));
+	xlisten(sck, 50);
 	sprintf(msg, "accepting UDP packets on addr:port %s:%d\n",
 		iface_addr, (int)listen_port);
 	log_message(LOG_FILE, msg);
-	return s;
+	return sck;
 }
 
 /*
@@ -415,8 +415,6 @@ int dnsd_main(int argc, char **argv)
 #endif
 
 	udps = listen_socket(listen_interface, port);
-	if (udps < 0)
-		exit(1);
 
 	while (1) {
 		fd_set fdset;
Index: networking/udhcp/dhcprelay.c
===================================================================
--- networking/udhcp/dhcprelay.c	(revision 16605)
+++ networking/udhcp/dhcprelay.c	(working copy)
@@ -169,7 +169,6 @@ static int init_sockets(char **client, i
 
 	/* talk to real server on bootps */
 	fds[0] = listen_socket(htonl(INADDR_ANY), 67, server);
-	if (fds[0] < 0) return -1;
 	*max_socket = fds[0];
 
 	/* array starts at 1 since server is 0 */
@@ -178,7 +177,6 @@ static int init_sockets(char **client, i
 	for (i=1; i < num_clients; i++) {
 		/* listen for clients on bootps */
 		fds[i] = listen_socket(htonl(INADDR_ANY), 67, client[i-1]);
-		if (fds[i] < 0) return -1;
 		if (fds[i] > *max_socket) *max_socket = fds[i];
 	}
 
@@ -321,8 +319,6 @@ int dhcprelay_main(int argc, char **argv
 	signal(SIGINT, dhcprelay_signal_handler);
 
 	num_sockets = init_sockets(clients, num_sockets, argv[2], fds, &max_socket);
-	if (num_sockets == -1)
-		bb_perror_msg_and_die("init_sockets() failed");
 
 	if (read_interface(argv[2], NULL, &gw_ip, NULL) == -1)
 		return 1;
Index: networking/udhcp/dhcpc.c
===================================================================
--- networking/udhcp/dhcpc.c	(revision 16602)
+++ networking/udhcp/dhcpc.c	(working copy)
@@ -304,10 +304,6 @@ int udhcpc_main(int argc, char *argv[])
 				fd = listen_socket(INADDR_ANY, CLIENT_PORT, client_config.interface);
 			else
 				fd = raw_socket(client_config.ifindex);
-			if (fd < 0) {
-				bb_perror_msg("FATAL: cannot listen on socket");
-				return 0;
-			}
 		}
 		max_fd = udhcp_sp_fd_set(&rfds, fd);
 
Index: networking/udhcp/dhcpd.c
===================================================================
--- networking/udhcp/dhcpd.c	(revision 16602)
+++ networking/udhcp/dhcpd.c	(working copy)
@@ -69,10 +69,8 @@ int udhcpd_main(int argc, char *argv[])
 	while (1) { /* loop until universe collapses */
 
 		if (server_socket < 0)
-			if ((server_socket = listen_socket(INADDR_ANY, SERVER_PORT, server_config.interface)) < 0) {
-				bb_perror_msg("FATAL: cannot create server socket");
-				return 2;
-			}
+			server_socket = listen_socket(INADDR_ANY, SERVER_PORT,
+					server_config.interface);
 
 		max_sock = udhcp_sp_fd_set(&rfds, server_socket);
 		if (server_config.auto_time) {
Index: networking/udhcp/clientsocket.c
===================================================================
--- networking/udhcp/clientsocket.c	(revision 16602)
+++ networking/udhcp/clientsocket.c	(working copy)
@@ -40,20 +40,12 @@ int raw_socket(int ifindex)
 	struct sockaddr_ll sock;
 
 	DEBUG("Opening raw socket on ifindex %d", ifindex);
-	fd = socket(PF_PACKET, SOCK_DGRAM, htons(ETH_P_IP));
-	if (fd < 0) {
-		bb_perror_msg("socket");
-		return -1;
-	}
+	fd = xsocket(PF_PACKET, SOCK_DGRAM, htons(ETH_P_IP));
 
 	sock.sll_family = AF_PACKET;
 	sock.sll_protocol = htons(ETH_P_IP);
 	sock.sll_ifindex = ifindex;
-	if (bind(fd, (struct sockaddr *) &sock, sizeof(sock)) < 0) {
-		bb_perror_msg("bind");
-		close(fd);
-		return -1;
-	}
+	xbind(fd, (struct sockaddr *) &sock, sizeof(sock));
 
 	return fd;
 }
Index: networking/udhcp/socket.c
===================================================================
--- networking/udhcp/socket.c	(revision 16602)
+++ networking/udhcp/socket.c	(working copy)
@@ -96,11 +96,7 @@ int listen_socket(uint32_t ip, int port,
 	int n = 1;
 
 	DEBUG("Opening listen socket on 0x%08x:%d %s", ip, port, inf);
-	fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
-	if (fd < 0) {
-		bb_perror_msg("socket");
-		return -1;
-	}
+	fd = xsocket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
 
 	memset(&addr, 0, sizeof(addr));
 	addr.sin_family = AF_INET;


_______________________________________________
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic