On Mon, Dec 13, 2021 at 11:25:02AM +0100, Florian Obser wrote:
> Replace struct member assignment with struct assignment to make the code
> more compact. No binary change (on amd64).
>
> OK? Or is there a reason not to do this?
Looks good to me and I also see no reason why this should not be done.
> diff --git dhcpleased.c dhcpleased.c
> index c8cc8e14d04..00bbffc1699 100644
> --- dhcpleased.c
> +++ dhcpleased.c
> @@ -785,16 +785,14 @@ configure_interface(struct imsg_configure_interface
> *imsg)
> if (ifa->ifa_addr->sa_family != AF_INET)
> continue;
>
> - addr.s_addr = ((struct sockaddr_in *)ifa->ifa_addr)
> - ->sin_addr.s_addr;
> - mask.s_addr = ((struct sockaddr_in *)ifa->ifa_netmask)
> - ->sin_addr.s_addr;
> + addr = ((struct sockaddr_in *)ifa->ifa_addr)->sin_addr;
> + mask = ((struct sockaddr_in *)ifa->ifa_netmask)->sin_addr;
>
> if (imsg->addr.s_addr == addr.s_addr) {
> if (imsg->mask.s_addr == mask.s_addr)
> found = 1;
> else {
> - req_sin_addr->sin_addr.s_addr = addr.s_addr;
> + req_sin_addr->sin_addr = addr;
> if (ioctl(ioctl_sock, SIOCDIFADDR, &ifaliasreq)
> == -1) {
> if (errno != EADDRNOTAVAIL)
> @@ -805,12 +803,12 @@ configure_interface(struct imsg_configure_interface
> *imsg)
> }
> }
>
> - req_sin_addr->sin_addr.s_addr = imsg->addr.s_addr;
> + req_sin_addr->sin_addr = imsg->addr;
> if (!found) {
> req_sin_mask = (struct sockaddr_in *)&ifaliasreq.ifra_mask;
> req_sin_mask->sin_family = AF_INET;
> req_sin_mask->sin_len = sizeof(*req_sin_mask);
> - req_sin_mask->sin_addr.s_addr = imsg->mask.s_addr;
> + req_sin_mask->sin_addr = imsg->mask;
> if (ioctl(ioctl_sock, SIOCAIFADDR, &ifaliasreq) == -1)
> log_warn("SIOCAIFADDR");
> }
> @@ -931,7 +929,7 @@ deconfigure_interface(struct imsg_configure_interface
> *imsg)
> req_sin_addr->sin_family = AF_INET;
> req_sin_addr->sin_len = sizeof(*req_sin_addr);
>
> - req_sin_addr->sin_addr.s_addr = imsg->addr.s_addr;
> + req_sin_addr->sin_addr = imsg->addr;
> if (ioctl(ioctl_sock, SIOCDIFADDR, &ifaliasreq) == -1) {
> if (errno != EADDRNOTAVAIL)
> log_warn("SIOCDIFADDR");
> @@ -948,7 +946,7 @@ configure_routes(uint8_t rtm_type, struct
> imsg_configure_interface *imsg)
> memset(&ifa, 0, sizeof(ifa));
> ifa.sin_family = AF_INET;
> ifa.sin_len = sizeof(ifa);
> - ifa.sin_addr.s_addr = imsg->addr.s_addr;
> + ifa.sin_addr = imsg->addr;
>
> memset(&dst, 0, sizeof(dst));
> dst.sin_family = AF_INET;
> @@ -965,9 +963,9 @@ configure_routes(uint8_t rtm_type, struct
> imsg_configure_interface *imsg)
> addrnet = imsg->addr.s_addr & imsg->mask.s_addr;
>
> for (i = 0; i < imsg->routes_len; i++) {
> - dst.sin_addr.s_addr = imsg->routes[i].dst.s_addr;
> - mask.sin_addr.s_addr = imsg->routes[i].mask.s_addr;
> - gw.sin_addr.s_addr = imsg->routes[i].gw.s_addr;
> + dst.sin_addr = imsg->routes[i].dst;
> + mask.sin_addr = imsg->routes[i].mask;
> + gw.sin_addr = imsg->routes[i].gw;
>
> if (gw.sin_addr.s_addr == INADDR_ANY) {
> /* direct route */
> @@ -988,8 +986,7 @@ configure_routes(uint8_t rtm_type, struct
> imsg_configure_interface *imsg)
> configure_route(rtm_type, imsg->if_index,
> imsg->rdomain, &gw, &mask, &ifa, NULL,
> RTF_CLONING);
> - mask.sin_addr.s_addr =
> - imsg->routes[i].mask.s_addr;
> + mask.sin_addr = imsg->routes[i].mask;
> }
>
> if (gw.sin_addr.s_addr == ifa.sin_addr.s_addr) {
> diff --git engine.c engine.c
> index 09867c595b4..60f81d6f3c8 100644
> --- engine.c
> +++ engine.c
> @@ -570,10 +570,10 @@ send_interface_info(struct dhcpleased_iface *iface,
> pid_t pid)
> strlcpy(cei.state, if_state_name[iface->state], sizeof(cei.state));
> memcpy(&cei.request_time, &iface->request_time,
> sizeof(cei.request_time));
> - cei.server_identifier.s_addr = iface->server_identifier.s_addr;
> - cei.dhcp_server.s_addr = iface->dhcp_server.s_addr;
> - cei.requested_ip.s_addr = iface->requested_ip.s_addr;
> - cei.mask.s_addr = iface->mask.s_addr;
> + cei.server_identifier = iface->server_identifier;
> + cei.dhcp_server = iface->dhcp_server;
> + cei.requested_ip = iface->requested_ip;
> + cei.mask = iface->mask;
> cei.routes_len = iface->routes_len;
> memcpy(cei.routes, iface->routes, sizeof(cei.routes));
> memcpy(cei.nameservers, iface->nameservers, sizeof(cei.nameservers));
> @@ -1169,9 +1169,9 @@ parse_dhcp(struct dhcpleased_iface *iface, struct
> imsg_dhcp *dhcp)
> "offered IP address", __func__);
> return;
> }
> - iface->server_identifier.s_addr = server_identifier.s_addr;
> - iface->dhcp_server.s_addr = server_identifier.s_addr;
> - iface->requested_ip.s_addr = dhcp_hdr->yiaddr.s_addr;
> + iface->server_identifier = server_identifier;
> + iface->dhcp_server = server_identifier;
> + iface->requested_ip = dhcp_hdr->yiaddr;
> state_transition(iface, IF_REQUESTING);
> break;
> case DHCPACK:
> @@ -1228,10 +1228,10 @@ parse_dhcp(struct dhcpleased_iface *iface, struct
> imsg_dhcp *dhcp)
> rebinding_time = lease_time - (lease_time / 8);
>
> clock_gettime(CLOCK_MONOTONIC, &iface->request_time);
> - iface->server_identifier.s_addr = server_identifier.s_addr;
> - iface->dhcp_server.s_addr = server_identifier.s_addr;
> - iface->requested_ip.s_addr = dhcp_hdr->yiaddr.s_addr;
> - iface->mask.s_addr = subnet_mask.s_addr;
> + iface->server_identifier = server_identifier;
> + iface->dhcp_server = server_identifier;
> + iface->requested_ip = dhcp_hdr->yiaddr;
> + iface->mask = subnet_mask;
> #ifndef SMALL
> if (iface_conf != NULL && iface_conf->ignore & IGN_ROUTES) {
> iface->routes_len = 0;
> @@ -1260,7 +1260,7 @@ parse_dhcp(struct dhcpleased_iface *iface, struct
> imsg_dhcp *dhcp)
> sizeof(iface->nameservers));
> }
>
> - iface->siaddr.s_addr = dhcp_hdr->siaddr.s_addr;
> + iface->siaddr = dhcp_hdr->siaddr;
>
> /* we made sure this is a string futher up */
> strnvis(iface->file, dhcp_hdr->file, sizeof(iface->file),
> @@ -1582,9 +1582,9 @@ send_configure_interface(struct dhcpleased_iface *iface)
> memset(&imsg, 0, sizeof(imsg));
> imsg.if_index = iface->if_index;
> imsg.rdomain = iface->rdomain;
> - imsg.addr.s_addr = iface->requested_ip.s_addr;
> - imsg.mask.s_addr = iface->mask.s_addr;
> - imsg.siaddr.s_addr = iface->siaddr.s_addr;
> + imsg.addr = iface->requested_ip;
> + imsg.mask = iface->mask;
> + imsg.siaddr = iface->siaddr;
> strlcpy(imsg.file, iface->file, sizeof(imsg.file));
> strlcpy(imsg.domainname, iface->domainname, sizeof(imsg.domainname));
> strlcpy(imsg.hostname, iface->hostname, sizeof(imsg.hostname));
> @@ -1621,9 +1621,9 @@ send_deconfigure_interface(struct dhcpleased_iface
> *iface)
>
> imsg.if_index = iface->if_index;
> imsg.rdomain = iface->rdomain;
> - imsg.addr.s_addr = iface->requested_ip.s_addr;
> - imsg.mask.s_addr = iface->mask.s_addr;
> - imsg.siaddr.s_addr = iface->siaddr.s_addr;
> + imsg.addr = iface->requested_ip;
> + imsg.mask = iface->mask;
> + imsg.siaddr = iface->siaddr;
> strlcpy(imsg.file, iface->file, sizeof(imsg.file));
> strlcpy(imsg.domainname, iface->domainname, sizeof(imsg.domainname));
> strlcpy(imsg.hostname, iface->hostname, sizeof(imsg.hostname));
> @@ -1650,9 +1650,9 @@ send_routes_withdraw(struct dhcpleased_iface *iface)
>
> imsg.if_index = iface->if_index;
> imsg.rdomain = iface->rdomain;
> - imsg.addr.s_addr = iface->requested_ip.s_addr;
> - imsg.mask.s_addr = iface->mask.s_addr;
> - imsg.siaddr.s_addr = iface->siaddr.s_addr;
> + imsg.addr = iface->requested_ip;
> + imsg.mask = iface->mask;
> + imsg.siaddr = iface->siaddr;
> strlcpy(imsg.file, iface->file, sizeof(imsg.file));
> strlcpy(imsg.domainname, iface->domainname, sizeof(imsg.domainname));
> strlcpy(imsg.hostname, iface->hostname, sizeof(imsg.hostname));
> diff --git frontend.c frontend.c
> index 57e60c57ee2..53b61cafb77 100644
> --- frontend.c
> +++ frontend.c
> @@ -890,10 +890,10 @@ void
> iface_data_from_imsg(struct iface* iface, struct imsg_req_dhcp *imsg)
> {
> iface->xid = imsg->xid;
> - iface->ciaddr.s_addr = imsg->ciaddr.s_addr;
> - iface->requested_ip.s_addr = imsg->requested_ip.s_addr;
> - iface->server_identifier.s_addr = imsg->server_identifier.s_addr;
> - iface->dhcp_server.s_addr = imsg->dhcp_server.s_addr;
> + iface->ciaddr = imsg->ciaddr;
> + iface->requested_ip = imsg->requested_ip;
> + iface->server_identifier = imsg->server_identifier;
> + iface->dhcp_server = imsg->dhcp_server;
> }
>
> ssize_t
> @@ -937,7 +937,7 @@ build_packet(uint8_t message_type, char *if_name,
> uint32_t xid,
> hdr->hops = 0;
> hdr->xid = xid;
> hdr->secs = 0;
> - hdr->ciaddr.s_addr = ciaddr->s_addr;
> + hdr->ciaddr = *ciaddr;
> memcpy(hdr->chaddr, hw_address, sizeof(*hw_address));
> p += sizeof(struct dhcp_hdr);
> memcpy(p, dhcp_cookie, sizeof(dhcp_cookie));
> @@ -1043,7 +1043,7 @@ udp_send_packet(struct iface *iface, uint8_t *packet,
> ssize_t len)
> memset(&to, 0, sizeof(to));
> to.sin_family = AF_INET;
> to.sin_len = sizeof(to);
> - to.sin_addr.s_addr = iface->dhcp_server.s_addr;
> + to.sin_addr = iface->dhcp_server;
> to.sin_port = ntohs(SERVER_PORT);
>
> if (sendto(iface->udpsock, packet, len, 0, (struct sockaddr *)&to,
> @@ -1080,7 +1080,7 @@ bpf_send_packet(struct iface *iface, uint8_t *packet,
> ssize_t len)
> ip.ip_ttl = 128;
> ip.ip_p = IPPROTO_UDP;
> ip.ip_sum = 0;
> - ip.ip_src.s_addr = 0;
> + ip.ip_src.s_addr = INADDR_ANY;
> ip.ip_dst.s_addr = INADDR_BROADCAST;
> ip.ip_sum = wrapsum(checksum((unsigned char *)&ip, sizeof(ip), 0));
> iov[iovcnt].iov_base = &ip;
>
> --
> I'm not entirely sure you are real.
>
--
:wq Claudio
