You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/03/19 14:39:13 UTC
[GitHub] [incubator-nuttx] antmerlino commented on a change in pull request #3100: net/tcp: correct the port byte order
antmerlino commented on a change in pull request #3100:
URL: https://github.com/apache/incubator-nuttx/pull/3100#discussion_r597731322
##########
File path: net/tcp/tcp_conn.c
##########
@@ -1133,42 +1133,49 @@ int tcp_connect(FAR struct tcp_conn_s *conn, FAR const struct sockaddr *addr)
net_lock();
+ /* Check if the local port has been bind() */
+
+ port = conn->lport;
+
+ if (port == 0)
+ {
#ifdef CONFIG_NET_IPv4
#ifdef CONFIG_NET_IPv6
- if (conn->domain == PF_INET)
+ if (conn->domain == PF_INET)
#endif
- {
- /* Select a port that is unique for this IPv4 local address (host
- * order).
- */
+ {
+ /* Select a port that is unique for this IPv4 local address (host
+ * order).
+ */
- port = tcp_selectport(PF_INET,
- (FAR const union ip_addr_u *)&conn->u.ipv4.laddr,
- ntohs(conn->lport));
- }
+ port = tcp_selectport(PF_INET,
+ (FAR const union ip_addr_u *)
+ &conn->u.ipv4.laddr, 0);
+ }
#endif /* CONFIG_NET_IPv4 */
#ifdef CONFIG_NET_IPv6
#ifdef CONFIG_NET_IPv4
- else
+ else
#endif
- {
- /* Select a port that is unique for this IPv6 local address (host
- * order).
- */
+ {
+ /* Select a port that is unique for this IPv6 local address (host
+ * order).
+ */
- port = tcp_selectport(PF_INET6,
- (FAR const union ip_addr_u *)conn->u.ipv6.laddr,
- ntohs(conn->lport));
- }
+ port = tcp_selectport(PF_INET6,
Review comment:
same as above
##########
File path: net/tcp/tcp_conn.c
##########
@@ -1133,42 +1133,49 @@ int tcp_connect(FAR struct tcp_conn_s *conn, FAR const struct sockaddr *addr)
net_lock();
+ /* Check if the local port has been bind() */
+
+ port = conn->lport;
+
+ if (port == 0)
+ {
#ifdef CONFIG_NET_IPv4
#ifdef CONFIG_NET_IPv6
- if (conn->domain == PF_INET)
+ if (conn->domain == PF_INET)
#endif
- {
- /* Select a port that is unique for this IPv4 local address (host
- * order).
- */
+ {
+ /* Select a port that is unique for this IPv4 local address (host
+ * order).
+ */
- port = tcp_selectport(PF_INET,
- (FAR const union ip_addr_u *)&conn->u.ipv4.laddr,
- ntohs(conn->lport));
- }
+ port = tcp_selectport(PF_INET,
Review comment:
Need to use htons() so that `port` is in network order for assignment to lport below.
##########
File path: net/tcp/tcp_conn.c
##########
@@ -285,19 +286,18 @@ static int tcp_selectport(uint8_t domain, FAR const union ip_addr_u *ipaddr,
do
{
/* Guess that the next available port number will be the one after
- * the last port number assigned.
+ * the last port number assigned. Make sure that the port number
+ * is within range.
*/
- portno = ++g_last_tcp_port;
-
- /* Make sure that the port number is within range */
-
- if (g_last_tcp_port >= 32000)
+ if (++g_last_tcp_port >= 32000)
{
g_last_tcp_port = 4096;
}
+
+ portno = htons(g_last_tcp_port);
Review comment:
This function, as documented, should return the port in host order. This change makes it so that when portno==0, the port will be returned in network order, which is wrong.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org