PTY: Add some encode to reduce the likely of misinterpreting the return value of the file open() method

drivers/serial/ptmx.c

@@ -255,10 +255,11 @@ static int ptmx_open(FAR struct file *filep)
   DEBUGASSERT(ret >= 0);  /* unlink() should never fail */
   UNUSED(ret);
 
-  /* Return the master file descriptor */
+  /* Return the encoded, master file descriptor */
 
   ptmx_semgive();
-  return fd;
+  DEBUGASSERT((unsigned)fd <= OPEN_MAXFD);
+  return (int)OPEN_SETFD(fd);
 
 errout_with_minor:
   ptmx_minor_free(minor);

fs/vfs/fs_open.c

@@ -234,17 +234,19 @@ int open(const char *path, int oflags, ...)
     }
 
 #ifdef CONFIG_PSEUDOTERM_SUSV1
-  /* If the return value from the open method is > 0, then we may assume that
-   * it is actually a file descriptor.  This kind of logic is only needed for
-   * /dev/ptmx:  When dev ptmx is opened, it does not return a file descriptor
-   * associated with the /dev/ptmx inode, but rather with the master device.
+  /* If the return value from the open method is > 0, then it may actually
+   * be an encoded file descriptor.  This kind of logic is currently only
+   * needed for /dev/ptmx:  When dev ptmx is opened, it does not return a
+   * file descriptor associated with the /dev/ptmx inode, but rather with
+   * the inode of master device created by the /dev/ptmx open method.
    *
-   * REVISIT: This is a kludge.  It is dangerous because (1) Zero is also a
-   * valid file descriptor, and (2) there could potentially be driver open
-   * methods that return values > 0 for normal success conditions.
+   * The encoding supports (a) returning file descriptor 0 (which really
+   * should not happen), and (b) avoiding confusion if some other open
+   * method returns a positive, non-zero value which is not a file
+   * descriptor.
    */
 
-  if (ret > 0)
+  if (OPEN_ISFD(ret))
     {
       /* Release file descriptor and inode that we allocated.  We don't
        * need those.
@@ -253,11 +255,11 @@ int open(const char *path, int oflags, ...)
       files_release(fd);
       inode_release(inode);
 
-      /* Instead, return the descriptor associated with the master side
-       * device.
+      /* Instead, decode and return the descriptor associated with the
+       * master side device.
        */
 
-      fd = ret;
+      fd = (int)OPEN_GETFD(ret);
     }
 #endif
 

include/nuttx/fs/fs.h

@@ -111,6 +111,35 @@
 #define DIRENT_SETPSEUDONODE(f) do (f) |= DIRENTFLAGS_PSEUDONODE; while (0)
 #define DIRENT_ISPSEUDONODE(f) (((f) & DIRENTFLAGS_PSEUDONODE) != 0)
 
+/* The struct file_operations open(0) normally returns zero on success and
+ * a negated errno value on failure.  There is one case, however, where
+ * the open method will redirect to another driver and return a file
+ * descriptor instead.
+ *
+ * This case is when SUSv1 pseudo-terminals are used (CONFIG_PSEUDOTERM_SUSV1=y).
+ * In this case, the output is encoded and decoded using these macros in 
+ * order to support (a) returning file descriptor 0 (which really should
+ * not happen), and (b) avoiding confusion if some other open method returns
+ * a positive, non-zero value which is not a file descriptor.
+ *
+ *   OPEN_ISFD(r) tests if the return value from the open method is
+ *     really a file descriptor.
+ *   OPEN_SETFD(f) is used by an implementation of the open() method
+ *     in order to encode a file descriptor in the return value.
+ *   OPEN_GETFD(r) is use by the upper level open() logic to decode
+ *     the file descriptor encoded in the return value.
+ *
+ * REVISIT: This only works for file descriptors in the in range 0-255.
+ */
+
+#define OPEN_MAGIC      0x4200
+#define OPEN_MASK       0x00ff
+#define OPEN_MAXFD      0x00ff
+
+#define OPEN_ISFD(r)    (((r) & ~OPEN_MASK) == OPEN_MAGIC)
+#define OPEN_SETFD(f)   ((f) | OPEN_MAGIC)
+#define OPEN_GETFD(r)   ((r) & OPEN_MASK)
+
 /****************************************************************************
  * Public Type Definitions
  ****************************************************************************/