casaroli commented on code in PR #20702:
URL: https://github.com/apache/nuttx/pull/20702#discussion_r4445406773
##########
net/local/local_recvmsg.c:
##########
@@ -255,38 +268,103 @@ psock_stream_recvfrom(FAR struct socket *psock, FAR void
*buf, size_t len,
if (flags & MSG_DONTWAIT)
{
int data_len = 0;
+
ret = file_ioctl(&conn->lc_infile, FIONREAD, &data_len);
if (ret < 0)
{
- return ret;
+ goto errout_with_lock;
}
if (data_len == 0)
{
- return -EAGAIN;
+ ret = -EAGAIN;
+ goto errout_with_lock;
}
}
- /* Read the packet */
+ /* Read into user iovec buffers */
- ret = psock_fifo_read(psock, buf, 0, &readlen, flags, true);
- if (ret < 0)
+ for (i = 0; i < msg->msg_iovlen; i++)
{
- return ret;
+ if (msg->msg_iov[i].iov_len == 0 ||
+ msg->msg_iov[i].iov_base == NULL)
+ {
+ continue;
+ }
+
+ iov_recv = 0;
+ while (iov_recv < msg->msg_iov[i].iov_len)
+ {
+ cur_flags = flags;
+
+ /* If data has already been received and MSG_WAITALL is not
+ * set, do not block waiting for more data.
+ */
+
+ if (total_recv > 0 && !(flags & MSG_WAITALL))
+ {
+ cur_flags |= MSG_DONTWAIT;
+ }
+
+ readlen = msg->msg_iov[i].iov_len - iov_recv;
+ ret = psock_fifo_read(psock,
+ (FAR uint8_t *)msg->msg_iov[i].iov_base +
+ iov_recv,
+ peek_offset, &readlen, cur_flags, true);
+ if (ret < 0)
+ {
+ break;
+ }
+
+ if (readlen == 0)
+ {
+ /* EOF */
+
+ break;
+ }
+
+ iov_recv += readlen;
+ total_recv += readlen;
+ if (flags & MSG_PEEK)
+ {
+ peek_offset += readlen;
+ }
+
+ /* If MSG_WAITALL is not set and we read less than requested,
+ * return what we have so far without blocking further.
+ */
+
+ if (!(flags & MSG_WAITALL))
+ {
+ break;
+ }
+ }
+
+ /* If an error occurred or EOF was reached, or if partial data was
+ * received without MSG_WAITALL, stop processing subsequent iovecs.
+ */
+
+ if (ret < 0 || readlen == 0 ||
+ (!(flags & MSG_WAITALL) && iov_recv < msg->msg_iov[i].iov_len))
+ {
+ break;
+ }
}
/* Return the address family */
- if (from)
+ if (from != NULL)
{
ret = local_getaddr(conn, from, fromlen);
Review Comment:
An error from psock_fifo_read() is lost when msg_name is set: the loop
breaks with ret < 0, then local_getaddr() sets ret to 0, and the function
returns 0 (end of file). Test: recvfrom() with an address buffer on an empty
O_NONBLOCK socket returns 0; master returns -1 with EAGAIN.
Fix: when total_recv == 0 and ret < 0, go to errout_with_lock before
local_getaddr(), as master returned the error.
##########
net/local/local_recvmsg.c:
##########
@@ -225,27 +222,43 @@ static void local_recvctl(FAR struct local_conn_s *conn,
#ifdef CONFIG_NET_LOCAL_STREAM
static inline ssize_t
-psock_stream_recvfrom(FAR struct socket *psock, FAR void *buf, size_t len,
- int flags, FAR struct sockaddr *from,
- FAR socklen_t *fromlen)
+psock_stream_recvfrom(FAR struct socket *psock, FAR struct msghdr *msg,
+ int flags)
{
FAR struct local_conn_s *conn = psock->s_conn;
- size_t readlen = len;
- int ret;
+ FAR struct sockaddr *from = msg->msg_name;
+ FAR socklen_t *fromlen = &msg->msg_namelen;
+ size_t peek_offset = 0;
+ ssize_t total_recv = 0;
+ size_t iov_recv;
+ size_t readlen;
+ ssize_t ret = 0;
+ int cur_flags;
+ int i;
+
+ /* Make sure that receive is multi-thread safe */
+
+ ret = nxmutex_lock(&conn->lc_recvlock);
Review Comment:
lc_recvlock stays locked while psock_fifo_read() waits for data, so
recv(MSG_DONTWAIT) waits for another thread's blocking recv() on the same
socket. Test: one thread blocked in recv(), a second thread's
recv(MSG_DONTWAIT) returns EAGAIN only after 1.2 s, when data comes; master
returns EAGAIN at once.
Fix: with MSG_DONTWAIT, take the lock with nxmutex_trylock() and return
-EAGAIN when it fails.
##########
net/local/local_recvmsg.c:
##########
@@ -255,38 +268,103 @@ psock_stream_recvfrom(FAR struct socket *psock, FAR void
*buf, size_t len,
if (flags & MSG_DONTWAIT)
{
int data_len = 0;
+
ret = file_ioctl(&conn->lc_infile, FIONREAD, &data_len);
if (ret < 0)
{
- return ret;
+ goto errout_with_lock;
}
if (data_len == 0)
{
- return -EAGAIN;
+ ret = -EAGAIN;
+ goto errout_with_lock;
}
}
- /* Read the packet */
+ /* Read into user iovec buffers */
- ret = psock_fifo_read(psock, buf, 0, &readlen, flags, true);
- if (ret < 0)
+ for (i = 0; i < msg->msg_iovlen; i++)
{
- return ret;
+ if (msg->msg_iov[i].iov_len == 0 ||
+ msg->msg_iov[i].iov_base == NULL)
+ {
+ continue;
+ }
+
+ iov_recv = 0;
+ while (iov_recv < msg->msg_iov[i].iov_len)
+ {
+ cur_flags = flags;
+
+ /* If data has already been received and MSG_WAITALL is not
+ * set, do not block waiting for more data.
+ */
+
+ if (total_recv > 0 && !(flags & MSG_WAITALL))
+ {
+ cur_flags |= MSG_DONTWAIT;
Review Comment:
When the data ends exactly at the end of an iov, recvmsg() blocks in the
read for the next iov. MSG_DONTWAIT in cur_flags does nothing here:
psock_fifo_read() uses the flags only for MSG_PEEK, and local_fifo_read() reads
the FIFO in its blocking mode. Test on qemu-armv8a:knsh: 8 bytes queued,
recvmsg() with two 8-byte iovs (no flags, or MSG_DONTWAIT) returns only after
the next write.
Fix: before the read into the next iov (total_recv > 0, no MSG_WAITALL), do
the FIONREAD check of line 268 and stop when it gives 0.
--
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]
