casaroli commented on code in PR #21113:
URL: https://github.com/apache/nuttx/pull/21113#discussion_r4469821665
##########
drivers/serial/serial.c:
##########
@@ -1684,105 +1730,133 @@ static ssize_t uart_writev(FAR struct file *filep,
FAR struct uio *uio)
nseg = 0;
}
- /* With no output processing, copy whole runs into the TX buffer at
- * once. The bulk copy caches the head index, so it is only used
- * when this thread is provably the sole producer: ECHO makes
- * uart_readv() echo into the same buffer, and on a console
- * uart_irqwrite() produces into it from interrupt context. A full
- * TX buffer falls through to uart_putxmitchar() below, which keeps
- * the canonical blocking, disconnect and O_NONBLOCK handling.
+ /* Number of contiguous bytes remaining in the current segment. */
+
+ nsent = seglen - nseg;
+
+ /* Locate the next character in this segment that requires output
+ * post-processing.
*/
-#ifdef CONFIG_SERIAL_TXBULK
- if ((dev->tc_oflag & OPOST) == 0 && (dev->tc_lflag & ECHO) == 0 &&
- !dev->isconsole)
+ if (opost && (dev->tc_oflag & OCRNL) != 0)
+ {
+ special = memchr(&segbuf[nseg], '\r', nsent);
+ }
+
+ if (opost && (dev->tc_oflag & (ONLCR | ONLRET)) != 0)
{
- ncopy = uart_putxmitbuf(dev, segbuf + nseg, seglen - nseg);
- if (ncopy > 0)
+ FAR const char *nl = memchr(&segbuf[nseg], '\n', nsent);
+
+ if (nl != NULL && (special == NULL || nl < special))
{
- nseg += ncopy;
- buflen -= ncopy;
- continue;
+ special = nl;
}
}
-#endif
- ch = segbuf[nseg];
- ret = OK;
+ if (special != NULL)
+ {
+ nsent = special - &segbuf[nseg];
+ }
- /* Do output post-processing */
+ /* Bulk-copy the run of ordinary bytes that precede the special
+ * character (or the rest of the segment when there is none).
+ */
- if ((dev->tc_oflag & OPOST) != 0)
+ if (nsent > 0)
{
- /* Mapping CR to NL? */
-
- if ((ch == '\r') && (dev->tc_oflag & OCRNL) != 0)
+ ret = uart_putxmitbuf(dev, &segbuf[nseg], nsent, oktoblock);
+ if (ret < 0)
{
- ch = '\n';
+ break;
}
- /* Are we interested in newline processing? */
-
- if ((ch == '\n') && (dev->tc_oflag & (ONLCR | ONLRET)) != 0)
+ nseg += ret;
+ buflen -= ret;
+ if (ret < nsent)
{
- ret = uart_putxmitchar(dev, '\r', oktoblock);
+ continue;
Review Comment:
A signal no longer ends a blocking write(). `uart_putxmitbuf()` returns the
partial count when the wait ends with a signal, and this `continue` calls it
again, so it waits again. sim:nsh, write() of 200000 bytes to a console that
nobody reads, SIGUSR1 after 3 s: base returns 16320 after 3 s, this PR returns
200000 after 12 s, when the host starts to read.
Fix: `break` here instead of `continue`. A short count only comes from a
signal, a disconnect or a full non-blocking buffer, and the code after the loop
returns the partial count. With this change the same test returns 16320 after 3
s.
##########
drivers/serial/serial.c:
##########
@@ -1669,8 +1700,23 @@ static ssize_t uart_writev(FAR struct file *filep, FAR
struct uio *uio)
*/
uart_disabletxint(dev);
+
+ /* Copy the payload into the TX buffer. Output post-processing only needs
+ * to translate CR (when OCRNL is set) and NL (when ONLCR/ONLRET is set);
+ * every other byte is copied verbatim. So within each iovec segment,
+ * locate the next byte that needs translation, bulk-copy everything before
+ * it with uart_putxmitbuffer(), then handle that one byte. When no
Review Comment:
This comment names `uart_putxmitbuffer()`, which does not exist. The title
("batch TX buffer fills with uart_putxmitbuffer") and the body ("It replaces
uart_putxmitbuf with uart_putxmitbuffer()") use the same name, and neither the
body nor the commit message says that CONFIG_SERIAL_TXBULK is removed.
Fix: use `uart_putxmitbuf()` in the comment, the title and the body, and say
in the commit message and in Impact that the option is gone.
##########
boards/arm/stm32h7/weact-stm32h750/configs/sdcard/defconfig:
##########
@@ -16,7 +16,6 @@ CONFIG_ARCH_CHIP_STM32=y
CONFIG_ARCH_CHIP_STM32H750VB=y
CONFIG_ARCH_CHIP_STM32H7=y
CONFIG_ARCH_CHIP_STM32H7_CORTEXM7=y
-CONFIG_ARCH_STACKDUMP=y
Review Comment:
3b2dd5cfa55 removed CONFIG_EXAMPLES_ALARM from this defconfig, which frees
about 2 KiB of text with GCC 13.2; this PR adds 192 bytes with GCC 13.2.
Fix: rebase on master and drop this commit if arm-stm32h7 still passes.
--
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]
