[haiku-commits] Change in haiku[master]: kernel/fifo: Properly support opening FIFOs in O_RDWR mode.

  • From: Gerrit <review@xxxxxxxxxxxxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Thu, 30 Jun 2022 16:16:22 +0000

From waddlesplash <waddlesplash@xxxxxxxxx>:

waddlesplash has uploaded this change for review. ( 
https://review.haiku-os.org/c/haiku/+/5427 ;)


Change subject: kernel/fifo: Properly support opening FIFOs in O_RDWR mode.
......................................................................

kernel/fifo: Properly support opening FIFOs in O_RDWR mode.

While pipe() only supports a read end and a write end, named FIFOs
are supposed to allow opening in read/write mode (which, indeed,
is something the Fish shell seems to require.)

The code that checks open_mode in the read/write routines was introduced
all the way back in 2003 (2469f26dfc618dac7853c0de146df7872e60623f).
At that time, the VFS did not do open_mode checks on FDs in the
I/O syscalls, so these doubled as permissions checks. Now, however,
the VFS does check that in common_user_io, so we can remove these
entirely and let things behave as they should.

Additionally, there is now also no reason to prevent select()ing
on both events for the same FD independent of open_mode, so
allow that too.
---
M src/system/kernel/fs/fifo.cpp
1 file changed, 16 insertions(+), 27 deletions(-)



  git pull ssh://git.haiku-os.org:22/haiku refs/changes/27/5427/1

diff --git a/src/system/kernel/fs/fifo.cpp b/src/system/kernel/fs/fifo.cpp
index a49b3f2..848b9ea 100644
--- a/src/system/kernel/fs/fifo.cpp
+++ b/src/system/kernel/fs/fifo.cpp
@@ -190,10 +190,8 @@
                        int32                           ReaderCount() const { 
return fReaderCount; }
                        int32                           WriterCount() const { 
return fWriterCount; }

-                       status_t                        Select(uint8 event, 
selectsync* sync,
-                                                                       int 
openMode);
-                       status_t                        Deselect(uint8 event, 
selectsync* sync,
-                                                                       int 
openMode);
+                       status_t                        Select(uint8 event, 
selectsync* sync);
+                       status_t                        Deselect(uint8 event, 
selectsync* sync);

                        void                            Dump(bool dumpData) 
const;
        static  int                                     Dump(int argc, char** 
argv);
@@ -634,7 +632,7 @@
 {
        MutexLocker locker(RequestLock());

-       if ((openMode & O_ACCMODE) == O_WRONLY)
+       if ((openMode & O_ACCMODE) == O_WRONLY || (openMode & O_ACCMODE) == 
O_RDWR)
                fWriterCount++;

        if ((openMode & O_ACCMODE) == O_RDONLY || (openMode & O_ACCMODE) == 
O_RDWR)
@@ -669,11 +667,12 @@
                        request->Notify(B_FILE_ERROR);
        }

-       if ((openMode & O_ACCMODE) == O_WRONLY && --fWriterCount == 0)
-               NotifyEndClosed(true);
+       if ((openMode & O_ACCMODE) == O_WRONLY || (openMode & O_ACCMODE) == 
O_RDWR) {
+               if (--fWriterCount == 0)
+                       NotifyEndClosed(true);
+       }

-       if ((openMode & O_ACCMODE) == O_RDONLY
-               || (openMode & O_ACCMODE) == O_RDWR) {
+       if ((openMode & O_ACCMODE) == O_RDONLY || (openMode & O_ACCMODE) == 
O_RDWR) {
                if (--fReaderCount == 0)
                        NotifyEndClosed(false);
        }
@@ -693,14 +692,14 @@


 status_t
-Inode::Select(uint8 event, selectsync* sync, int openMode)
+Inode::Select(uint8 event, selectsync* sync)
 {
        bool writer = true;
        select_sync_pool** pool;
-       if ((openMode & O_RWMASK) == O_RDONLY) {
+       if (event == B_SELECT_READ) {
                pool = &fReadSelectSyncPool;
                writer = false;
-       } else if ((openMode & O_RWMASK) == O_WRONLY) {
+       } else if (event == B_SELECT_WRITE) {
                pool = &fWriteSelectSyncPool;
        } else
                return B_NOT_ALLOWED;
@@ -727,12 +726,12 @@


 status_t
-Inode::Deselect(uint8 event, selectsync* sync, int openMode)
+Inode::Deselect(uint8 event, selectsync* sync)
 {
        select_sync_pool** pool;
-       if ((openMode & O_RWMASK) == O_RDONLY) {
+       if (event == B_SELECT_READ) {
                pool = &fReadSelectSyncPool;
-       } else if ((openMode & O_RWMASK) == O_WRONLY) {
+       } else if (event == B_SELECT_WRITE) {
                pool = &fWriteSelectSyncPool;
        } else
                return B_NOT_ALLOWED;
@@ -938,9 +937,6 @@

        MutexLocker locker(inode->RequestLock());

-       if ((cookie->open_mode & O_RWMASK) != O_RDONLY)
-               return B_NOT_ALLOWED;
-
        if (inode->IsActive() && inode->WriterCount() == 0) {
                // as long there is no writer, and the pipe is empty,
                // we always just return 0 to indicate end of file
@@ -987,9 +983,6 @@

        MutexLocker locker(inode->RequestLock());

-       if ((cookie->open_mode & O_RWMASK) != O_WRONLY)
-               return B_NOT_ALLOWED;
-
        size_t length = *_length;
        if (length == 0)
                return B_OK;
@@ -1141,15 +1134,13 @@
 fifo_select(fs_volume* _volume, fs_vnode* _node, void* _cookie,
        uint8 event, selectsync* sync)
 {
-       file_cookie* cookie = (file_cookie*)_cookie;
-
        TRACE("fifo_select(vnode = %p)\n", _node);
        Inode* inode = (Inode*)_node->private_node;
        if (!inode)
                return B_ERROR;

        MutexLocker locker(inode->RequestLock());
-       return inode->Select(event, sync, cookie->open_mode);
+       return inode->Select(event, sync);
 }
 

@@ -1157,15 +1148,13 @@
 fifo_deselect(fs_volume* _volume, fs_vnode* _node, void* _cookie,
        uint8 event, selectsync* sync)
 {
-       file_cookie* cookie = (file_cookie*)_cookie;
-
        TRACE("fifo_deselect(vnode = %p)\n", _node);
        Inode* inode = (Inode*)_node->private_node;
        if (inode == NULL)
                return B_ERROR;

        MutexLocker locker(inode->RequestLock());
-       return inode->Deselect(event, sync, cookie->open_mode);
+       return inode->Deselect(event, sync);
 }



--
To view, visit https://review.haiku-os.org/c/haiku/+/5427
To unsubscribe, or for help writing mail filters, visit 
https://review.haiku-os.org/settings

Gerrit-Project: haiku
Gerrit-Branch: master
Gerrit-Change-Id: Ib3cba76d18e91b7c00d1695e8d29dd47cae06b79
Gerrit-Change-Number: 5427
Gerrit-PatchSet: 1
Gerrit-Owner: waddlesplash <waddlesplash@xxxxxxxxx>
Gerrit-MessageType: newchange

Other related posts:

  • » [haiku-commits] Change in haiku[master]: kernel/fifo: Properly support opening FIFOs in O_RDWR mode. - Gerrit