diff --git a/drivers/serial/ptmx.c b/drivers/serial/ptmx.c index 2099328a5a1..dd6bd9fd3c5 100644 --- a/drivers/serial/ptmx.c +++ b/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); diff --git a/fs/vfs/fs_open.c b/fs/vfs/fs_open.c index 462aac6734e..08a3f24f647 100644 --- a/fs/vfs/fs_open.c +++ b/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 diff --git a/include/nuttx/fs/fs.h b/include/nuttx/fs/fs.h index 5b6cfdc31eb..03f62f976a6 100644 --- a/include/nuttx/fs/fs.h +++ b/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 ****************************************************************************/