fbd2918f5

Denis Vlasenko

2007-04-07

FAILED Duration: 14s

Subject

udhcp: MAC_BCAST_ADDR and blank_chaddr are in fact constant, move to rodata. a few global variables reduced to smallints function old new delta add_lease 75 227 +152 static.blank_chaddr

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

networking/udhcp/common.c +2 -0 networking/udhcp/common.h +3 -3 networking/udhcp/dhcpc.c +21 -15 networking/udhcp/dhcpd.h +2 -81 networking/udhcp/leases.c +35 -33 networking/udhcp/options.h +74 -0 networking/udhcp/packet.c +1 -1 networking/udhcp/script.c +30 -22 networking/udhcp/serverpacket.c +1 -1

networking/udhcp/common.c+2 -0

@@ -15,6 +15,8 @@
	#include "common.h"


+	const uint8_t MAC_BCAST_ADDR[6] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
+
	long uptime(void)
	{
	struct sysinfo info;

networking/udhcp/common.h+3 -3

@@ -14,8 +14,7 @@

	#define DEFAULT_SCRIPT "/usr/share/udhcpc/default.script"

-	#define COMBINED_BINARY
-
+	extern const uint8_t MAC_BCAST_ADDR[6]; /* six all-ones */

	/* packet.h */

@@ -52,7 +51,8 @@ int udhcp_get_packet(struct dhcpMessage *packet, int fd);
	uint16_t udhcp_checksum(void *addr, int count);
	int udhcp_raw_packet(struct dhcpMessage *payload,
	uint32_t source_ip, int source_port,
-	uint32_t dest_ip, int dest_port, uint8_t *dest_arp, int ifindex);
+	uint32_t dest_ip, int dest_port,
+	const uint8_t *dest_arp, int ifindex);
	int udhcp_kernel_packet(struct dhcpMessage *payload,
	uint32_t source_ip, int source_port,
	uint32_t dest_ip, int dest_port);

networking/udhcp/dhcpc.c+21 -15

@@ -17,22 +17,23 @@
	#include "options.h"


-	static int state;
	/* Something is definitely wrong here. IPv4 addresses
	* in variables of type long?? BTW, we use inet_ntoa()
	* in the code. Manpage says that struct in_addr has a member of type long (!)
	* which holds IPv4 address, and the struct is passed by value (!!)
	*/
+	static unsigned long timeout;
	static unsigned long requested_ip; /* = 0 */
	static uint32_t server_addr;
-	static unsigned long timeout;
	static int packet_num; /* = 0 */
-	static int fd = -1;
+	static int sockfd = -1;

	#define LISTEN_NONE 0
	#define LISTEN_KERNEL 1
	#define LISTEN_RAW 2
-	static int listen_mode;
+	static smallint listen_mode;
+
+	static smallint state;

	struct client_config_t client_config;

@@ -42,8 +43,10 @@ static void change_mode(int new_mode)
	{
	DEBUG("entering %s listen mode",
	new_mode ? (new_mode == 1 ? "kernel" : "raw") : "none");
-	if (fd >= 0) close(fd);
-	fd = -1;
+	if (sockfd >= 0) {
+	close(sockfd);
+	sockfd = -1;
+	}
	listen_mode = new_mode;
	}

@@ -111,6 +114,7 @@ static void client_background(void)
	* If that will be properly disabled for NOMMU, client_background()
	* will work on NOMMU too */
	#else
+	// chdir(/) is problematic. Imagine that e.g. pidfile name is RELATIVE! what will unlink do then, eh?
	bb_daemonize(DAEMON_CHDIR_ROOT);
	logmode &= ~LOGMODE_STDIO;
	#endif
@@ -289,13 +293,13 @@ int udhcpc_main(int argc, char **argv)
	tv.tv_sec = timeout - uptime();
	tv.tv_usec = 0;

-	if (listen_mode != LISTEN_NONE && fd < 0) {
+	if (listen_mode != LISTEN_NONE && sockfd < 0) {
	if (listen_mode == LISTEN_KERNEL)
-	fd = listen_socket(INADDR_ANY, CLIENT_PORT, client_config.interface);
+	sockfd = listen_socket(INADDR_ANY, CLIENT_PORT, client_config.interface);
	else
-	fd = raw_socket(client_config.ifindex);
+	sockfd = raw_socket(client_config.ifindex);
	}
-	max_fd = udhcp_sp_fd_set(&rfds, fd);
+	max_fd = udhcp_sp_fd_set(&rfds, sockfd);

	if (tv.tv_sec > 0) {
	DEBUG("Waiting on select...");
@@ -342,7 +346,8 @@ int udhcpc_main(int argc, char **argv)
	packet_num++;
	} else {
	/* timed out, go back to init state */
-	if (state == RENEW_REQUESTED) udhcp_run_script(NULL, "deconfig");
+	if (state == RENEW_REQUESTED)
+	udhcp_run_script(NULL, "deconfig");
	state = INIT_SELECTING;
	timeout = now;
	packet_num = 0;
@@ -393,12 +398,12 @@ int udhcpc_main(int argc, char **argv)
	timeout = 0x7fffffff;
	break;
	}
-	} else if (retval > 0 && listen_mode != LISTEN_NONE && FD_ISSET(fd, &rfds)) {
+	} else if (retval > 0 && listen_mode != LISTEN_NONE && FD_ISSET(sockfd, &rfds)) {
	/* a packet is ready, read it */

	if (listen_mode == LISTEN_KERNEL)
-	len = udhcp_get_packet(&packet, fd);
-	else len = get_raw_packet(&packet, fd);
+	len = udhcp_get_packet(&packet, sockfd);
+	else len = get_raw_packet(&packet, sockfd);

	if (len == -1 && errno != EINTR) {
	DEBUG("error on read, %s, reopening socket", strerror(errno));
@@ -418,7 +423,8 @@ int udhcpc_main(int argc, char **argv)
	continue;
	}

-	if ((message = get_option(&packet, DHCP_MESSAGE_TYPE)) == NULL) {
+	message = get_option(&packet, DHCP_MESSAGE_TYPE);
+	if (message == NULL) {
	bb_error_msg("cannot get option from packet - ignoring");
	continue;
	}

networking/udhcp/dhcpd.h+2 -81

@@ -14,81 +14,6 @@
	/* where to find the DHCP server configuration file */
	#define DHCPD_CONF_FILE "/etc/udhcpd.conf"

-	/*****************************************************************/
-	/* Do not modify below here unless you know what you are doing!! */
-	/*****************************************************************/
-
-	/* DHCP protocol -- see RFC 2131 */
-	#define SERVER_PORT 67
-	#define CLIENT_PORT 68
-
-	#define DHCP_MAGIC 0x63825363
-
-	/* DHCP option codes (partial list) */
-	#define DHCP_PADDING 0x00
-	#define DHCP_SUBNET 0x01
-	#define DHCP_TIME_OFFSET 0x02
-	#define DHCP_ROUTER 0x03
-	#define DHCP_TIME_SERVER 0x04
-	#define DHCP_NAME_SERVER 0x05
-	#define DHCP_DNS_SERVER 0x06
-	#define DHCP_LOG_SERVER 0x07
-	#define DHCP_COOKIE_SERVER 0x08
-	#define DHCP_LPR_SERVER 0x09
-	#define DHCP_HOST_NAME 0x0c
-	#define DHCP_BOOT_SIZE 0x0d
-	#define DHCP_DOMAIN_NAME 0x0f
-	#define DHCP_SWAP_SERVER 0x10
-	#define DHCP_ROOT_PATH 0x11
-	#define DHCP_IP_TTL 0x17
-	#define DHCP_MTU 0x1a
-	#define DHCP_BROADCAST 0x1c
-	#define DHCP_NTP_SERVER 0x2a
-	#define DHCP_WINS_SERVER 0x2c
-	#define DHCP_REQUESTED_IP 0x32
-	#define DHCP_LEASE_TIME 0x33
-	#define DHCP_OPTION_OVER 0x34
-	#define DHCP_MESSAGE_TYPE 0x35
-	#define DHCP_SERVER_ID 0x36
-	#define DHCP_PARAM_REQ 0x37
-	#define DHCP_MESSAGE 0x38
-	#define DHCP_MAX_SIZE 0x39
-	#define DHCP_T1 0x3a
-	#define DHCP_T2 0x3b
-	#define DHCP_VENDOR 0x3c
-	#define DHCP_CLIENT_ID 0x3d
-	#define DHCP_FQDN 0x51
-
-	#define DHCP_END 0xFF
-
-
-	#define BOOTREQUEST 1
-	#define BOOTREPLY 2
-
-	#define ETH_10MB 1
-	#define ETH_10MB_LEN 6
-
-	#define DHCPDISCOVER 1
-	#define DHCPOFFER 2
-	#define DHCPREQUEST 3
-	#define DHCPDECLINE 4
-	#define DHCPACK 5
-	#define DHCPNAK 6
-	#define DHCPRELEASE 7
-	#define DHCPINFORM 8
-
-	#define BROADCAST_FLAG 0x8000
-
-	#define OPTION_FIELD 0
-	#define FILE_FIELD 1
-	#define SNAME_FIELD 2
-
-	/* miscellaneous defines */
-	#define MAC_BCAST_ADDR (uint8_t *) "\xff\xff\xff\xff\xff\xff"
-	#define OPT_CODE 0
-	#define OPT_LEN 1
-	#define OPT_DATA 2
-
	struct option_set {
	uint8_t *data;
	struct option_set *next;
@@ -140,13 +65,9 @@ struct dhcpOfferedAddr {
	uint32_t expires; /* host order */
	};

-	extern uint8_t blank_chaddr[];
-
-	void clear_lease(uint8_t *chaddr, uint32_t yiaddr);
-	struct dhcpOfferedAddr add_lease(uint8_t chaddr, uint32_t yiaddr, unsigned long lease);
+	struct dhcpOfferedAddr add_lease(const uint8_t chaddr, uint32_t yiaddr, unsigned long lease);
	int lease_expired(struct dhcpOfferedAddr *lease);
-	struct dhcpOfferedAddr *oldest_expired_lease(void);
-	struct dhcpOfferedAddr find_lease_by_chaddr(uint8_t chaddr);
+	struct dhcpOfferedAddr find_lease_by_chaddr(const uint8_t chaddr);
	struct dhcpOfferedAddr *find_lease_by_yiaddr(uint32_t yiaddr);
	uint32_t find_address(int check_expired);

networking/udhcp/leases.c+35 -33

@@ -8,25 +8,41 @@
	#include "dhcpd.h"


-	uint8_t blank_chaddr[] = {[0 ... 15] = 0};
+	/* Find the oldest expired lease, NULL if there are no expired leases */
+	static struct dhcpOfferedAddr *oldest_expired_lease(void)
+	{
+	struct dhcpOfferedAddr *oldest = NULL;
+	unsigned long oldest_lease = time(0);
+	unsigned i;
+
+
+	for (i = 0; i < server_config.max_leases; i++)
+	if (oldest_lease > leases[i].expires) {
+	oldest_lease = leases[i].expires;
+	oldest = &(leases[i]);
+	}
+	return oldest;
+	}
+

	/* clear every lease out that chaddr OR yiaddr matches and is nonzero */
-	void clear_lease(uint8_t *chaddr, uint32_t yiaddr)
+	static void clear_lease(const uint8_t *chaddr, uint32_t yiaddr)
	{
-	unsigned int i, j;
+	unsigned i, j;

	for (j = 0; j < 16 && !chaddr[j]; j++);

	for (i = 0; i < server_config.max_leases; i++)
-	if ((j != 16 && !memcmp(leases[i].chaddr, chaddr, 16)) \|\|
-	(yiaddr && leases[i].yiaddr == yiaddr)) {
+	if ((j != 16 && memcmp(leases[i].chaddr, chaddr, 16) != 0)
+	\|\| (yiaddr && leases[i].yiaddr == yiaddr)
+	) {
	memset(&(leases[i]), 0, sizeof(struct dhcpOfferedAddr));
	}
	}


	/* add a lease into the table, clearing out any old ones */
-	struct dhcpOfferedAddr add_lease(uint8_t chaddr, uint32_t yiaddr, unsigned long lease)
+	struct dhcpOfferedAddr add_lease(const uint8_t chaddr, uint32_t yiaddr, unsigned long lease)
	{
	struct dhcpOfferedAddr *oldest;

@@ -52,31 +68,14 @@ int lease_expired(struct dhcpOfferedAddr *lease)
	}


-	/* Find the oldest expired lease, NULL if there are no expired leases */
-	struct dhcpOfferedAddr *oldest_expired_lease(void)
-	{
-	struct dhcpOfferedAddr *oldest = NULL;
-	unsigned long oldest_lease = time(0);
-	unsigned int i;
-
-
-	for (i = 0; i < server_config.max_leases; i++)
-	if (oldest_lease > leases[i].expires) {
-	oldest_lease = leases[i].expires;
-	oldest = &(leases[i]);
-	}
-	return oldest;
-
-	}
-
-
	/* Find the first lease that matches chaddr, NULL if no match */
-	struct dhcpOfferedAddr find_lease_by_chaddr(uint8_t chaddr)
+	struct dhcpOfferedAddr find_lease_by_chaddr(const uint8_t chaddr)
	{
-	unsigned int i;
+	unsigned i;

	for (i = 0; i < server_config.max_leases; i++)
-	if (!memcmp(leases[i].chaddr, chaddr, 16)) return &(leases[i]);
+	if (!memcmp(leases[i].chaddr, chaddr, 16))
+	return &(leases[i]);

	return NULL;
	}
@@ -85,10 +84,11 @@ struct dhcpOfferedAddr find_lease_by_chaddr(uint8_t chaddr)
	/* Find the first lease that matches yiaddr, NULL is no match */
	struct dhcpOfferedAddr *find_lease_by_yiaddr(uint32_t yiaddr)
	{
-	unsigned int i;
+	unsigned i;

	for (i = 0; i < server_config.max_leases; i++)
-	if (leases[i].yiaddr == yiaddr) return &(leases[i]);
+	if (leases[i].yiaddr == yiaddr)
+	return &(leases[i]);

	return NULL;
	}
@@ -97,6 +97,8 @@ struct dhcpOfferedAddr *find_lease_by_yiaddr(uint32_t yiaddr)
	/* check is an IP is taken, if it is, add it to the lease table */
	static int check_ip(uint32_t addr)
	{
+	static const uint8_t blank_chaddr[16]; /* 16 zero bytes */
+
	struct in_addr temp;

	if (arpping(addr, server_config.server, server_config.arp, server_config.interface) == 0) {
@@ -105,7 +107,8 @@ static int check_ip(uint32_t addr)
	inet_ntoa(temp), server_config.conflict_time);
	add_lease(blank_chaddr, addr, server_config.conflict_time);
	return 1;
-	} else return 0;
+	}
+	return 0;
	}


@@ -125,7 +128,7 @@ uint32_t find_address(int check_expired)
	/* ie, 192.168.55.255 */
	if ((addr & 0xFF) == 0xFF) continue;

-	/* Only do if it isn't an assigned as a static lease */
+	/* Only do if it isn't assigned as a static lease */
	if (!reservedIp(server_config.static_leases, htonl(addr))) {

	/* lease is not taken */
@@ -133,11 +136,10 @@ uint32_t find_address(int check_expired)
	lease = find_lease_by_yiaddr(ret);

	/* no lease or it expired and we are checking for expired leases */
-	if ( (!lease \|\| (check_expired && lease_expired(lease)))
+	if ((!lease \|\| (check_expired && lease_expired(lease)))
	&& /* and it isn't on the network */ !check_ip(ret)
	) {
	return ret;
-	break;
	}
	}
	}

networking/udhcp/options.h+74 -0

@@ -23,6 +23,80 @@ enum {
	#define OPTION_REQ 0x10 /* have the client request this option */
	#define OPTION_LIST 0x20 /* There can be a list of 1 or more of these */

+	/*****************************************************************/
+	/* Do not modify below here unless you know what you are doing!! */
+	/*****************************************************************/
+
+	/* DHCP protocol -- see RFC 2131 */
+	#define SERVER_PORT 67
+	#define CLIENT_PORT 68
+
+	#define DHCP_MAGIC 0x63825363
+
+	/* DHCP option codes (partial list) */
+	#define DHCP_PADDING 0x00
+	#define DHCP_SUBNET 0x01
+	#define DHCP_TIME_OFFSET 0x02
+	#define DHCP_ROUTER 0x03
+	#define DHCP_TIME_SERVER 0x04
+	#define DHCP_NAME_SERVER 0x05
+	#define DHCP_DNS_SERVER 0x06
+	#define DHCP_LOG_SERVER 0x07
+	#define DHCP_COOKIE_SERVER 0x08
+	#define DHCP_LPR_SERVER 0x09
+	#define DHCP_HOST_NAME 0x0c
+	#define DHCP_BOOT_SIZE 0x0d
+	#define DHCP_DOMAIN_NAME 0x0f
+	#define DHCP_SWAP_SERVER 0x10
+	#define DHCP_ROOT_PATH 0x11
+	#define DHCP_IP_TTL 0x17
+	#define DHCP_MTU 0x1a
+	#define DHCP_BROADCAST 0x1c
+	#define DHCP_NTP_SERVER 0x2a
+	#define DHCP_WINS_SERVER 0x2c
+	#define DHCP_REQUESTED_IP 0x32
+	#define DHCP_LEASE_TIME 0x33
+	#define DHCP_OPTION_OVER 0x34
+	#define DHCP_MESSAGE_TYPE 0x35
+	#define DHCP_SERVER_ID 0x36
+	#define DHCP_PARAM_REQ 0x37
+	#define DHCP_MESSAGE 0x38
+	#define DHCP_MAX_SIZE 0x39
+	#define DHCP_T1 0x3a
+	#define DHCP_T2 0x3b
+	#define DHCP_VENDOR 0x3c
+	#define DHCP_CLIENT_ID 0x3d
+	#define DHCP_FQDN 0x51
+
+	#define DHCP_END 0xFF
+
+
+	#define BOOTREQUEST 1
+	#define BOOTREPLY 2
+
+	#define ETH_10MB 1
+	#define ETH_10MB_LEN 6
+
+	#define DHCPDISCOVER 1
+	#define DHCPOFFER 2
+	#define DHCPREQUEST 3
+	#define DHCPDECLINE 4
+	#define DHCPACK 5
+	#define DHCPNAK 6
+	#define DHCPRELEASE 7
+	#define DHCPINFORM 8
+
+	#define BROADCAST_FLAG 0x8000
+
+	#define OPTION_FIELD 0
+	#define FILE_FIELD 1
+	#define SNAME_FIELD 2
+
+	/* miscellaneous defines */
+	#define OPT_CODE 0
+	#define OPT_LEN 1
+	#define OPT_DATA 2
+
	struct dhcp_option {
	char name[12];
	char flags;

networking/udhcp/packet.c+1 -1

@@ -112,7 +112,7 @@ uint16_t udhcp_checksum(void *addr, int count)
	void BUG_sizeof_struct_udp_dhcp_packet_must_be_576(void);
	int udhcp_raw_packet(struct dhcpMessage *payload,
	uint32_t source_ip, int source_port,
-	uint32_t dest_ip, int dest_port, uint8_t *dest_arp, int ifindex)
+	uint32_t dest_ip, int dest_port, const uint8_t *dest_arp, int ifindex)
	{
	int fd;
	int result;

networking/udhcp/script.c+30 -22

@@ -9,7 +9,6 @@
	*/

	#include "common.h"
-	#include "dhcpd.h"
	#include "dhcpc.h"
	#include "options.h"

@@ -77,7 +76,7 @@ static char alloc_fill_opts(uint8_t option, const struct dhcp_option *type_p)
	switch (type) {
	case OPTION_IP_PAIR:
	dest += sprintip(dest, "", option);
-	*(dest++) = '/';
+	*dest++ = '/';
	option += 4;
	optlen = 4;
	case OPTION_IP: /* Works regardless of host byte order. */
@@ -132,34 +131,42 @@ static char *fill_envp(struct dhcpMessage packet)
	int num_options = 0;
	int i, j;
	char **envp;
+	char *var;
	uint8_t *temp;
	struct in_addr subnet;
	char over = 0;

-	if (packet == NULL)
-	num_options = 0;
-	else {
-	for (i = 0; dhcp_options[i].code; i++)
+	if (packet) {
+	for (i = 0; dhcp_options[i].code; i++) {
	if (get_option(packet, dhcp_options[i].code)) {
	num_options++;
	if (dhcp_options[i].code == DHCP_SUBNET)
	num_options++; /* for mton */
	}
-	if (packet->siaddr) num_options++;
-	if ((temp = get_option(packet, DHCP_OPTION_OVER)))
+	}
+	if (packet->siaddr)
+	num_options++;
+	temp = get_option(packet, DHCP_OPTION_OVER);
+	if (temp)
	over = *temp;
-	if (!(over & FILE_FIELD) && packet->file[0]) num_options++;
-	if (!(over & SNAME_FIELD) && packet->sname[0]) num_options++;
+	if (!(over & FILE_FIELD) && packet->file[0])
+	num_options++;
+	if (!(over & SNAME_FIELD) && packet->sname[0])
+	num_options++;
	}

	envp = xzalloc(sizeof(char ) (num_options + 5));
	j = 0;
	envp[j++] = xasprintf("interface=%s", client_config.interface);
-	envp[j++] = xasprintf("PATH=%s",
-	getenv("PATH") ? : "/bin:/usr/bin:/sbin:/usr/sbin");
-	envp[j++] = xasprintf("HOME=%s", getenv("HOME") ? : "/");
+	var = getenv("PATH");
+	if (var)
+	envp[j++] = xasprintf("PATH=%s", var);
+	var = getenv("HOME");
+	if (var)
+	envp[j++] = xasprintf("HOME=%s", var);

-	if (packet == NULL) return envp;
+	if (packet == NULL)
+	return envp;

	envp[j] = xmalloc(sizeof("ip=255.255.255.255"));
	sprintip(envp[j++], "ip=", (uint8_t *) &packet->yiaddr);
@@ -206,19 +213,20 @@ void udhcp_run_script(struct dhcpMessage packet, const char name)
	DEBUG("vfork'ing and execle'ing %s", client_config.script);

	envp = fill_envp(packet);
+
	/* call script */
+	// can we use wait4pid(spawn(...)) here?
	pid = vfork();
-	if (pid) {
-	waitpid(pid, NULL, 0);
-	for (curr = envp; curr; curr++) free(curr);
-	free(envp);
-	return;
-	} else if (pid == 0) {
+	if (pid < 0) return;
+	if (pid == 0) {
	/* close fd's? */
	/* exec script */
	execle(client_config.script, client_config.script,
	name, NULL, envp);
-	bb_perror_msg("script %s failed", client_config.script);
-	exit(1);
+	bb_perror_msg_and_die("script %s failed", client_config.script);
	}
+	waitpid(pid, NULL, 0);
+	for (curr = envp; *curr; curr++)
+	free(*curr);
+	free(envp);
	}

networking/udhcp/serverpacket.c+1 -1

@@ -38,7 +38,7 @@ static int send_packet_to_relay(struct dhcpMessage *payload)
	/* send a packet to a specific arp address and ip address by creating our own ip packet */
	static int send_packet_to_client(struct dhcpMessage *payload, int force_broadcast)
	{
-	uint8_t *chaddr;
+	const uint8_t *chaddr;
	uint32_t ciaddr;

	if (force_broadcast) {