casaroli commented on code in PR #21158:
URL: https://github.com/apache/nuttx/pull/21158#discussion_r4435094012


##########
drivers/serial/pty.c:
##########
@@ -618,19 +617,94 @@ static ssize_t pty_write(FAR struct file *filep,
 
   if ((dev->pd_oflag & OPOST) != 0)
     {
-      /* We will transfer one byte at a time, making the appropriae
-       * translations.  Specifically not handled:
+      /* Output post-processing is enabled.  Locate the next byte that needs
+       * translation (CR under OCRNL, NL under ONLCR/ONLRET) or that is a tty
+       * control character (SIGINT/SIGTSTP), write the run of ordinary bytes
+       * before it to the sink in a single call, then handle that byte.
        *
+       * Specifically not handled:
        *   OXTABS - primarily a full-screen terminal optimisation
        *   ONOEOT - Unix interoperability hack
        *   OLCUC  - Not specified by POSIX
        *   ONOCR  - low-speed interactive optimisation
        */
 
-      ntotal = 0;
-      for (i = 0; i < len; i++)
+      FAR const char *special;
+      size_t nbulk;
+
+      while (ntotal < len)
         {
-          ch = *buffer++;
+          special = NULL;
+
+          if ((dev->pd_oflag & OCRNL) != 0)
+            {
+              special = memchr(&buffer[ntotal], '\r', len - ntotal);
+            }
+
+          if ((dev->pd_oflag & (ONLCR | ONLRET)) != 0)
+            {
+              FAR const char *nl;
+
+              nl = memchr(&buffer[ntotal], '\n', len - ntotal);
+              if (nl != NULL && (special == NULL || nl < special))
+                {
+                  special = nl;
+                }
+            }
+
+#if defined(CONFIG_TTY_SIGINT) || defined(CONFIG_TTY_SIGTSTP)
+          if (pid > 0 && (dev->pd_lflag & ISIG) != 0)
+            {
+              FAR const char *si;
+
+#ifdef CONFIG_TTY_SIGINT
+              si = memchr(&buffer[ntotal], CONFIG_TTY_SIGINT_CHAR,

Review Comment:
   Master compares with dev->pd_devpair->pp_vintr and pp_vsusp, which TCSETS 
sets from c_cc, and skips _POSIX_VDISABLE (81b320b9724). This PR uses 
CONFIG_TTY_SIGINT_CHAR and CONFIG_TTY_SIGTSTP_CHAR, so it conflicts, and after 
a rebase that keeps them tcsetattr() cannot change or disable these characters.
   
   Fix: rebase, and search for pp_vintr and pp_vsusp (only when they are not 
_POSIX_VDISABLE).



##########
drivers/serial/pty.c:
##########
@@ -1174,9 +1253,10 @@ int pty_register2(int minor, bool susv1)
   devpair->pp_master.pd_devpair = devpair;
   devpair->pp_master.pd_master  = true;
   devpair->pp_master.pd_oflag   = OPOST | OCRNL;
+  devpair->pp_master.pd_lflag   = ISIG;

Review Comment:
   TCSETS stores c_lflag only in pp_slave.pd_lflag (pty.c:882), so the ISIG set 
here on the master side never changes, and the new `(dev->pd_lflag & ISIG)` 
test is always true for writes to the master.
   
   Fix: test devpair->pp_slave.pd_lflag, the terminal settings of the pair.



-- 
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]

Reply via email to