| Message ID | 1455711269-12929-12-git-send-email-janusz.dziedzic@tieto.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Wed, Feb 17, 2016 at 01:14:14PM +0100, Janusz Dziedzic wrote: > Setup ctrl_interface name. This should be used > to check interface <-> udp port assigment. > diff --git a/wpa_supplicant/ctrl_iface_udp.c b/wpa_supplicant/ctrl_iface_udp.c > @@ -363,6 +363,7 @@ struct ctrl_iface_priv * > wpa_supplicant_ctrl_iface_init(struct wpa_supplicant *wpa_s) > + char port_str[40] = "udp:9877"; What is the point of setting the "udp:9877" value here? > + /* Setup correct ctrl_interface */ > + os_snprintf(port_str, sizeof(port_str), "udp:%d", port); Wouldn't this unconditionally replace that "udp:9877" value? > + if (wpa_s->conf->ctrl_interface) > + os_free(wpa_s->conf->ctrl_interface); os_free(NULL) is fine, so please no "if (ptr)" checks before os_free() calls. > + wpa_s->conf->ctrl_interface = os_strdup(port_str); Should there be a check to verify that os_strdup() succeeds here?
On 20 February 2016 at 16:24, Jouni Malinen <j@w1.fi> wrote: > On Wed, Feb 17, 2016 at 01:14:14PM +0100, Janusz Dziedzic wrote: >> Setup ctrl_interface name. This should be used >> to check interface <-> udp port assigment. > >> diff --git a/wpa_supplicant/ctrl_iface_udp.c b/wpa_supplicant/ctrl_iface_udp.c >> @@ -363,6 +363,7 @@ struct ctrl_iface_priv * >> wpa_supplicant_ctrl_iface_init(struct wpa_supplicant *wpa_s) > >> + char port_str[40] = "udp:9877"; > > What is the point of setting the "udp:9877" value here? > >> + /* Setup correct ctrl_interface */ >> + os_snprintf(port_str, sizeof(port_str), "udp:%d", port); > > Wouldn't this unconditionally replace that "udp:9877" value? > >> + if (wpa_s->conf->ctrl_interface) >> + os_free(wpa_s->conf->ctrl_interface); > > os_free(NULL) is fine, so please no "if (ptr)" checks before os_free() > calls. > >> + wpa_s->conf->ctrl_interface = os_strdup(port_str); > > Should there be a check to verify that os_strdup() succeeds here? > Will fix this in next version. > -- > Jouni Malinen PGP id EFC895FA
diff --git a/wpa_supplicant/ctrl_iface_udp.c b/wpa_supplicant/ctrl_iface_udp.c index 8ab8ee8..734a393 100644 --- a/wpa_supplicant/ctrl_iface_udp.c +++ b/wpa_supplicant/ctrl_iface_udp.c @@ -363,6 +363,7 @@ struct ctrl_iface_priv * wpa_supplicant_ctrl_iface_init(struct wpa_supplicant *wpa_s) { struct ctrl_iface_priv *priv; + char port_str[40] = "udp:9877"; int port = WPA_CTRL_IFACE_PORT; char *pos; #ifdef CONFIG_CTRL_IFACE_UDP_IPV6 @@ -380,9 +381,6 @@ wpa_supplicant_ctrl_iface_init(struct wpa_supplicant *wpa_s) priv->sock = -1; os_get_random(priv->cookie, COOKIE_LEN); - if (wpa_s->conf->ctrl_interface == NULL) - return priv; - pos = os_strstr(wpa_s->conf->ctrl_interface, "udp:"); if (pos) { pos += 4; @@ -424,12 +422,18 @@ try_again: #endif /* CONFIG_CTRL_IFACE_UDP_IPV6 */ if (bind(priv->sock, (struct sockaddr *) &addr, sizeof(addr)) < 0) { port--; - if ((WPA_CTRL_IFACE_PORT - port) < WPA_CTRL_IFACE_PORT_LIMIT && !pos) + if ((WPA_CTRL_IFACE_PORT - port) < WPA_CTRL_IFACE_PORT_LIMIT) goto try_again; wpa_printf(MSG_ERROR, "bind(AF_INET): %s", strerror(errno)); goto fail; } + /* Setup correct ctrl_interface */ + os_snprintf(port_str, sizeof(port_str), "udp:%d", port); + if (wpa_s->conf->ctrl_interface) + os_free(wpa_s->conf->ctrl_interface); + wpa_s->conf->ctrl_interface = os_strdup(port_str); + #ifdef CONFIG_CTRL_IFACE_UDP_REMOTE wpa_msg(wpa_s, MSG_DEBUG, "ctrl_iface_init UDP port: %d", port); #endif /* CONFIG_CTRL_IFACE_UDP_REMOTE */
Setup ctrl_interface name. This should be used to check interface <-> udp port assigment. Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com> --- wpa_supplicant/ctrl_iface_udp.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)