casaroli commented on code in PR #20702:
URL: https://github.com/apache/nuttx/pull/20702#discussion_r4456899132
##########
net/local/local_recvmsg.c:
##########
@@ -225,68 +222,188 @@ 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 */
+
+ if ((flags & MSG_DONTWAIT) != 0 ||
+ _SS_ISNONBLOCK(conn->lc_conn.s_flags))
+ {
+ ret = nxmutex_trylock(&conn->lc_recvlock);
+ if (ret < 0)
+ {
+ return -EAGAIN;
+ }
+ }
+ else
+ {
+ ret = nxmutex_lock(&conn->lc_recvlock);
+ if (ret < 0)
+ {
+ return ret;
+ }
+ }
/* Verify that this is a connected peer socket */
if (conn->lc_state != LOCAL_STATE_CONNECTED)
{
nerr("ERROR: not connected\n");
- return -ENOTCONN;
+ ret = -ENOTCONN;
+ goto errout_with_lock;
}
/* Check shutdown state */
if (conn->lc_infile.f_inode == NULL)
{
- return 0;
+ ret = 0;
+ goto errout_with_lock;
}
/* If it is non-blocking mode, the data in fifo is 0 and
* returns directly
*/
- if (flags & MSG_DONTWAIT)
+ if ((flags & MSG_DONTWAIT) != 0 ||
+ _SS_ISNONBLOCK(conn->lc_conn.s_flags))
{
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;
+ }
+
+ /* If data has already been received and MSG_WAITALL is not
+ * set, check if more data is available in the FIFO. If not,
+ * do not block waiting for more data in subsequent iovs.
+ */
+
+ if (total_recv > 0 && !(flags & MSG_WAITALL))
+ {
+ int data_len = 0;
+ int ioret;
+
+ ioret = file_ioctl(&conn->lc_infile, FIONREAD, &data_len);
+ if (ioret < 0 || data_len == 0)
+ {
+ break;
+ }
+ }
+
+ 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:
`cur_flags |= MSG_DONTWAIT` does nothing: psock_fifo_read() uses the flags
only for MSG_PEEK, and the FIONREAD check at line 313 is what stops the read
now.
Fix: remove cur_flags and pass `flags` to psock_fifo_read().
##########
net/local/local_recvmsg.c:
##########
@@ -225,68 +222,188 @@ 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 */
+
+ if ((flags & MSG_DONTWAIT) != 0 ||
+ _SS_ISNONBLOCK(conn->lc_conn.s_flags))
+ {
+ ret = nxmutex_trylock(&conn->lc_recvlock);
+ if (ret < 0)
+ {
+ return -EAGAIN;
+ }
+ }
+ else
+ {
+ ret = nxmutex_lock(&conn->lc_recvlock);
+ if (ret < 0)
+ {
+ return ret;
+ }
+ }
/* Verify that this is a connected peer socket */
if (conn->lc_state != LOCAL_STATE_CONNECTED)
{
nerr("ERROR: not connected\n");
- return -ENOTCONN;
+ ret = -ENOTCONN;
+ goto errout_with_lock;
}
/* Check shutdown state */
if (conn->lc_infile.f_inode == NULL)
{
- return 0;
+ ret = 0;
+ goto errout_with_lock;
}
/* If it is non-blocking mode, the data in fifo is 0 and
* returns directly
*/
- if (flags & MSG_DONTWAIT)
+ if ((flags & MSG_DONTWAIT) != 0 ||
+ _SS_ISNONBLOCK(conn->lc_conn.s_flags))
Review Comment:
An O_NONBLOCK socket now gets -EAGAIN at end of file: when the peer has
closed, FIONREAD is 0 too, so recv() returns -1 EAGAIN for ever instead of 0.
Master does this check for MSG_DONTWAIT only; for O_NONBLOCK the FIFO is open
with O_NONBLOCK, so the read returns -EAGAIN when it is empty and 0 at end of
file. Test on qemu-armv8a:knsh: socketpair(), O_NONBLOCK on one end, close() of
the other, recv() returns -1 EAGAIN (master: 0); with 4 bytes queued before the
close: 4, then -1 EAGAIN (master: 4, then 0).
Fix: keep `if (flags & MSG_DONTWAIT)` here as master has it; the trylock for
O_NONBLOCK at line 241 can stay.
--
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]
