ricardgb commented on code in PR #3658:
URL: https://github.com/apache/nuttx-apps/pull/3658#discussion_r4382244208
##########
netutils/telnetd/telnetd_daemon.c:
##########
@@ -125,6 +125,27 @@ int telnetd_daemon(FAR const struct telnetd_config_s
*config)
goto errout;
}
+ /* If the daemon was started without standard streams (e.g. spawned by
+ * nsh_telnetstart() before a USB console exists), socket() may have
+ * returned a descriptor in 0..2. The "go silent" close(0)..close(2)
Review Comment:
Done: commit 1 now anchors any free 0..2 slot to `/dev/null` before
`socket()`, and the accept loop re-anchors 0..2 to `/dev/null` instead of
closing them. That replaces the `F_DUPFD_CLOEXEC` relocation.
##########
netutils/telnetd/telnetd_daemon.c:
##########
@@ -224,17 +223,33 @@ int telnetd_daemon(FAR const struct telnetd_config_s
*config)
acceptsd = accept4(listensd, &addr.generic, &addrlen, SOCK_CLOEXEC);
if (acceptsd < 0)
{
- /* Just continue if a signal was received */
+ int err = errno;
- if (errno == EINTR)
+ /* A bad listen socket can never recover - exiting loudly beats
+ * spinning forever while serving nothing.
+ */
+
+ if (err == EBADF || err == ENOTSOCK || err == EINVAL ||
+ err == EOPNOTSUPP)
{
- continue;
+ nerr("ERROR: accept failed fatally: %d\n", err);
+ goto errout_with_socket;
}
- else
+
+ /* Everything else is transient: a peer that reset before the
+ * accept completed (a port scan, nc -z, ECONNABORTED), a
+ * signal, or the interface bouncing. None of those may take
+ * the daemon down; the pause keeps a persistent transient
+ * (interface down) from busy-spinning.
+ */
+
+ nerr("ERROR: accept failed: %d\n", err);
+ if (err != EINTR)
{
- nerr("ERROR: accept failed: %d\n", errno);
- goto errout_with_socket;
+ usleep(100 * 1000);
Review Comment:
Removed.
##########
netutils/telnetd/telnetd_daemon.c:
##########
@@ -224,17 +223,33 @@ int telnetd_daemon(FAR const struct telnetd_config_s
*config)
acceptsd = accept4(listensd, &addr.generic, &addrlen, SOCK_CLOEXEC);
if (acceptsd < 0)
{
- /* Just continue if a signal was received */
+ int err = errno;
- if (errno == EINTR)
+ /* A bad listen socket can never recover - exiting loudly beats
+ * spinning forever while serving nothing.
+ */
+
+ if (err == EBADF || err == ENOTSOCK || err == EINVAL ||
+ err == EOPNOTSUPP)
Review Comment:
Agreed. `accept4()` errors now just log (except `EINTR`) and `continue`; the
fatal-errno list is gone.
##########
netutils/telnetd/telnetd_daemon.c:
##########
@@ -126,12 +126,11 @@ int telnetd_daemon(FAR const struct telnetd_config_s
*config)
}
/* If the daemon was started without standard streams (e.g. spawned by
- * nsh_telnetstart() before a USB console device exists), socket() may
- * have returned a descriptor in 0..2. The "go silent"
- * close(0)..close(2) at the top of the accept loop below would then
- * destroy the listen socket: every subsequent accept4() fails and the
- * daemon serves nothing. Move the descriptor above the
- * standard-stream range.
+ * nsh_telnetstart() before a USB console exists), socket() may have
Review Comment:
This no longer applies: commit 1 was rewritten, so the comment it touched is
gone and commit 2 only changes the error paths.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
