dhcpv4: don't copy reqopts around

Ok, this is a bit weird, dhcpv4_handle_msg() passes reqopts to dhcpv4_lease(),
which stashes a copy of the reqopts in the struct dhcp_assignment, and then
dhcpv4_handle_msg() uses the stashed copy instead.

The only reason a copy is made seems to be so that the requested options can
later be reported via ubus (in "ubus call dhcp ipv4leases"), but it is hard to
see a use-case for that (the DHCP server has already replied to the client by
the time the lease is available over ubus) and we don't do anything similar for
DHCPv6, so remove the extra copying.

Signed-off-by: David Härdeman <[email protected]>
Link: https://github.com/openwrt/odhcpd/pull/278
Signed-off-by: Álvaro Fernández Rojas <[email protected]>

diff --git a/src/dhcpv4.c b/src/dhcpv4.c
index 6b24ae1883193c990d5593cc26ad209c0000fb06..236dbcbefdbc7290b4765f3cccac2b7e58d7afce 100644 (file)
--- a/src/dhcpv4.c
+++ b/src/dhcpv4.c
@@ -398,7 +398,7 @@ static struct dhcp_assignment*
 dhcpv4_lease(struct interface *iface, enum dhcpv4_msg msg, const uint8_t *mac,
             const uint32_t reqaddr, uint32_t *leasetime, const char *hostname,
             const size_t hostname_len, const bool accept_fr_nonce, bool *incl_fr_opt,
-            uint32_t *fr_serverid, const uint8_t *reqopts, const size_t reqopts_len)
+            uint32_t *fr_serverid)
 {
        struct dhcp_assignment *a = find_assignment_by_hwaddr(iface, mac);
        struct lease *l = config_find_lease_by_mac(mac);
@@ -503,14 +503,6 @@ dhcpv4_lease(struct interface *iface, enum dhcpv4_msg msg, const uint8_t *mac,
                                        }
                                }
 
-                               if (reqopts_len > 0) {
-                                       a->reqopts = realloc(a->reqopts, reqopts_len);
-                                       if (a->reqopts) {
-                                               memcpy(a->reqopts, reqopts, reqopts_len);
-                                               a->reqopts_len = reqopts_len;
-                                       }
-                               }
-
                                if (!(a->flags & OAF_BOUND)) {
                                        a->accept_fr_nonce = accept_fr_nonce;
                                        *incl_fr_opt = accept_fr_nonce;
@@ -650,8 +642,10 @@ void dhcpv4_handle_msg(void *addr, void *data, size_t len,
                                return;
                        break;
                case DHCPV4_OPT_REQOPTS:
-                       req_opts = opt->data;
-                       req_opts_len = opt->len;
+                       if (opt->len > 0) {
+                               req_opts = opt->data;
+                               req_opts_len = opt->len;
+                       }
                        break;
                case DHCPV4_OPT_USER_CLASS:
                        if (iface->filter_class) {
@@ -694,8 +688,7 @@ void dhcpv4_handle_msg(void *addr, void *data, size_t len,
        case DHCPV4_MSG_RELEASE:
                a = dhcpv4_lease(iface, req_msg, req->chaddr, req_addr,
                                 &req_leasetime, req_hostname, req_hostname_len,
-                                req_accept_fr, &incl_fr_opt, &fr_serverid,
-                                req_opts, req_opts_len);
+                                req_accept_fr, &incl_fr_opt, &fr_serverid);
                break;
        case DHCPV4_MSG_INFORM:
                break;
@@ -838,8 +831,8 @@ void dhcpv4_handle_msg(void *addr, void *data, size_t len,
                dhcpv4_put(&reply, &cursor, DHCPV4_OPT_DNSSERVER,
                                4 * iface->dhcpv4_dns_cnt, iface->dhcpv4_dns);
 
-       for (size_t opt = 0; a && opt < a->reqopts_len; opt++) {
-               switch (a->reqopts[opt]) {
+       for (size_t opt = 0; a && opt < req_opts_len; opt++) {
+               switch (req_opts[opt]) {
                case DHCPV4_OPT_NTPSERVER:
                        dhcpv4_put(&reply, &cursor, DHCPV4_OPT_NTPSERVER,
                                   4 * iface->dhcpv4_ntp_cnt, iface->dhcpv4_ntp);

diff --git a/src/odhcpd.h b/src/odhcpd.h
index 8d5fa195d29a0f7d6229dd11577045132f688f32..7d2636521499bb1891a1a207e44df317ea6ca4a8 100644 (file)
--- a/src/odhcpd.h
+++ b/src/odhcpd.h
@@ -265,8 +265,6 @@ struct dhcp_assignment {
        unsigned int flags;
        uint32_t leasetime;
        char *hostname;
-       uint8_t *reqopts;
-       size_t reqopts_len;
 
        uint8_t hwaddr[6];
 
@@ -456,7 +454,6 @@ inline static void free_assignment(struct dhcp_assignment *a)
                a->dhcp_free_cb(a);
 
        free(a->hostname);
-       free(a->reqopts);
        free(a);
 }
 

diff --git a/src/ubus.c b/src/ubus.c
index 47282da204e0365e57e1e21c56d51eff1390dcf1..1a1672667bb3eae9b2cd24032308f90e8319132c 100644 (file)
--- a/src/ubus.c
+++ b/src/ubus.c
@@ -50,14 +50,6 @@ static int handle_dhcpv4_leases(struct ubus_context *ctx, _unused struct ubus_ob
                        blobmsg_add_string(&b, "hostname", (c->hostname) ? c->hostname : "");
                        blobmsg_add_u8(&b, "accept-reconf", c->accept_fr_nonce);
 
-                       if (c->reqopts_len > 0) {
-                               buf = blobmsg_alloc_string_buffer(&b, "reqopts", c->reqopts_len * 4 + 1);
-                               for (size_t i = 0; i < c->reqopts_len; i++)
-                                       buf += snprintf(buf, 5, "%" PRIu8 ",", c->reqopts[i]);
-                               buf[-1] = '\0';
-                               blobmsg_add_string_buffer(&b);
-                       }
-
                        m = blobmsg_open_array(&b, "flags");
                        if (c->flags & OAF_BOUND)
                                blobmsg_add_string(&b, NULL, "bound");