Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers/serial/serial.c: only adapt readv api to avoid block on secound read in readv() #15604

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+74 −74
Open

drivers/serial/serial.c: only adapt readv api to avoid block on secound read in readv() #15604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6e509ce
fs/vfs: skip uio logic if only one iovcnt in slot
anchao Jan 19, 2025
57926a9
drivers/serial/serial.c: only adapt readv api to avoid block on secou…
anchao Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 72 additions & 72 deletions drivers/serial/serial.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,6 @@ static int uart_putxmitchar(FAR uart_dev_t *dev, int ch,
static inline ssize_t uart_irqwrite(FAR uart_dev_t *dev,
FAR const char *buffer,
size_t buflen);
static inline ssize_t uart_irqwritev(FAR uart_dev_t *dev,
FAR struct uio *uio);
static int uart_tcdrain(FAR uart_dev_t *dev,
bool cancelable, clock_t timeout);

Expand All @@ -112,8 +110,12 @@ static int uart_tcsendbreak(FAR uart_dev_t *dev,

static int uart_open(FAR struct file *filep);
static int uart_close(FAR struct file *filep);
static ssize_t uart_read(FAR struct file *filep,
FAR char *buffer, size_t buflen);
static ssize_t uart_write(FAR struct file *filep,
FAR const char *buffer,
size_t buflen);
static ssize_t uart_readv(FAR struct file *filep, FAR struct uio *uio);
static ssize_t uart_writev(FAR struct file *filep, FAR struct uio *uio);
static int uart_ioctl(FAR struct file *filep,
int cmd, unsigned long arg);
static int uart_poll(FAR struct file *filep,
Expand All @@ -140,15 +142,15 @@ static const struct file_operations g_serialops =
{
uart_open, /* open */
uart_close, /* close */
NULL, /* read */
NULL, /* write */
uart_read, /* read */
uart_write, /* write */
NULL, /* seek */
uart_ioctl, /* ioctl */
NULL, /* mmap */
NULL, /* truncate */
uart_poll, /* poll */
uart_readv, /* readv */
uart_writev /* writev */
NULL /* writev */
#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
, uart_unlink /* unlink */
#endif
Expand Down Expand Up @@ -440,50 +442,6 @@ static inline ssize_t uart_irqwrite(FAR uart_dev_t *dev,
return buflen;
}

/****************************************************************************
* Name: uart_irqwritev
****************************************************************************/

static inline ssize_t uart_irqwritev(FAR uart_dev_t *dev,
FAR struct uio *uio)
{
ssize_t error = 0;
ssize_t total = 0;
int iovcnt = uio->uio_iovcnt;
int i;

for (i = 0; i < iovcnt; i++)
{
const struct iovec *iov = &uio->uio_iov[i];
if (iov->iov_len == 0)
{
continue;
}

ssize_t written = uart_irqwrite(dev, iov->iov_base, iov->iov_len);
if (written < 0)
{
error = written;
break;
}

if (SSIZE_MAX - total < written)
{
error = -EOVERFLOW;
break;
}

total += written;
}

if (error != 0 && total == 0)
{
return error;
}

return total;
}

/****************************************************************************
* Name: uart_tcdrain
*
Expand Down Expand Up @@ -890,10 +848,11 @@ static int uart_close(FAR struct file *filep)
}

/****************************************************************************
* Name: uart_readv
* Name: uart_read_internal
****************************************************************************/

static ssize_t uart_readv(FAR struct file *filep, FAR struct uio *uio)
static ssize_t uart_read_internal(FAR struct file *filep, FAR char *buffer,
size_t buflen, bool nonblock)
{
FAR struct inode *inode = filep->f_inode;
FAR uart_dev_t *dev = inode->i_private;
Expand All @@ -904,7 +863,6 @@ static ssize_t uart_readv(FAR struct file *filep, FAR struct uio *uio)
#endif
irqstate_t flags;
ssize_t recvd = 0;
ssize_t buflen;
bool echoed = false;
int16_t tail;
char ch;
Expand All @@ -928,8 +886,7 @@ static ssize_t uart_readv(FAR struct file *filep, FAR struct uio *uio)
* data from the end of the buffer.
*/

buflen = uio->uio_resid;
while (recvd < buflen)
while ((size_t)recvd < buflen)
{
#ifdef CONFIG_SERIAL_REMOVABLE
/* If the removable device is no longer connected, refuse to read any
Expand Down Expand Up @@ -1005,8 +962,7 @@ static ssize_t uart_readv(FAR struct file *filep, FAR struct uio *uio)
{
if (recvd > 0)
{
static const char zero = '\0';
uio_copyfrom(uio, recvd, &zero, 1);
*buffer-- = '\0';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't this break BS/DEL on iov boundaries?

recvd--;
if (dev->tc_lflag & ECHO)
{
Expand Down Expand Up @@ -1035,7 +991,7 @@ static ssize_t uart_readv(FAR struct file *filep, FAR struct uio *uio)

/* Store the received character */

uio_copyfrom(uio, recvd, &ch, 1);
*buffer++ = ch;
recvd++;

if (dev->tc_lflag & ECHO)
Expand Down Expand Up @@ -1105,7 +1061,7 @@ static ssize_t uart_readv(FAR struct file *filep, FAR struct uio *uio)
* return what we have.
*/

else if ((filep->f_oflags & O_NONBLOCK) != 0)
else if (nonblock)
{
/* If nothing was transferred, then return the -EAGAIN
* error (not zero which means end of file).
Expand Down Expand Up @@ -1147,7 +1103,7 @@ static ssize_t uart_readv(FAR struct file *filep, FAR struct uio *uio)
* wait.
*/

else if ((filep->f_oflags & O_NONBLOCK) != 0)
else if (nonblock)
{
/* Break out of the loop returning -EAGAIN */

Expand Down Expand Up @@ -1370,24 +1326,70 @@ static ssize_t uart_readv(FAR struct file *filep, FAR struct uio *uio)
#endif

nxmutex_unlock(&dev->recv.lock);
if (recvd >= 0)
return recvd;
}

static ssize_t uart_read(FAR struct file *filep,
FAR char *buffer, size_t buflen)
{
return uart_read_internal(filep, buffer, buflen,
!!(filep->f_oflags & O_NONBLOCK));
}

/****************************************************************************
* Name: uart_readv
****************************************************************************/

static ssize_t uart_readv(FAR struct file *filep, FAR struct uio *uio)
{
FAR const struct iovec *iov = uio->uio_iov;
int iovcnt = uio->uio_iovcnt;
bool nonblock = !!(filep->f_oflags & O_NONBLOCK);
ssize_t ntotal;
ssize_t nread;
int i;

for (i = 0, ntotal = 0; i < iovcnt; i++)
{
uio_advance(uio, recvd);
nread = uart_read_internal(filep, iov[i].iov_base,
iov[i].iov_len, nonblock);
if (nread < 0)
{
if (ntotal > 0)
{
break;
}

return nread;
}

ntotal += nread;

/* Check for a parital success condition, including an end-of-file */

if (nread < iov[i].iov_len)
{
break;
}

/* set nonblock flag after first read */

nonblock = true;
}

return recvd;
return ntotal;
}

/****************************************************************************
* Name: uart_writev
* Name: uart_write
****************************************************************************/

static ssize_t uart_writev(FAR struct file *filep, FAR struct uio *uio)
static ssize_t uart_write(FAR struct file *filep, FAR const char *buffer,
xiaoxiang781216 marked this conversation as resolved.
Show resolved Hide resolved
size_t buflen)
{
FAR struct inode *inode = filep->f_inode;
FAR uart_dev_t *dev = inode->i_private;
ssize_t nwritten;
ssize_t buflen;
ssize_t nwritten = buflen;
bool oktoblock;
int ret;
char ch;
Expand All @@ -1413,14 +1415,12 @@ static ssize_t uart_writev(FAR struct file *filep, FAR struct uio *uio)
#endif

flags = enter_critical_section();
ret = uart_irqwritev(dev, uio);
ret = uart_irqwrite(dev, buffer, buflen);
leave_critical_section(flags);

return ret;
}

buflen = nwritten = uio->uio_resid;

/* Only one user can access dev->xmit.head at a time */

ret = nxmutex_lock(&dev->xmit.lock);
Expand Down Expand Up @@ -1460,9 +1460,9 @@ static ssize_t uart_writev(FAR struct file *filep, FAR struct uio *uio)
*/

uart_disabletxint(dev);
for (; buflen; uio_advance(uio, 1), buflen--)
for (; buflen; buflen--)
{
uio_copyto(uio, 0, &ch, 1);
ch = *buffer++;
ret = OK;

/* Do output post-processing */
Expand Down
2 changes: 1 addition & 1 deletion fs/vfs/fs_read.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ ssize_t file_readv(FAR struct file *filep,

else if (inode != NULL && inode->u.i_ops)
{
if (inode->u.i_ops->readv)
if (iovcnt > 1 && inode->u.i_ops->readv)
{
struct uio uio;

Expand Down
2 changes: 1 addition & 1 deletion fs/vfs/fs_write.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ ssize_t file_writev(FAR struct file *filep,
inode = filep->f_inode;
if (inode != NULL && inode->u.i_ops)
{
if (inode->u.i_ops->writev)
if (iovcnt > 1 && inode->u.i_ops->writev)
{
struct uio uio;

Expand Down
Loading