Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix write syscall #296

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix write syscall #296

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions src/syscall/systemio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ class SystemIO : public QObject {
static constexpr int O_TRUNC = 0x00000400; // 1024
static constexpr int O_EXCL = 0x00000800; // 2048

enum Permission {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enum class

WRITE_PERMS,
READ_PERMS,
};

// //////////////////////////////////////////////////////////////////////////////
// Maintain information on files in use. The index to the arrays is the "file
// descriptor."
Expand Down Expand Up @@ -184,18 +189,16 @@ class SystemIO : public QObject {
});
}

// Determine whether a given fd is already in use with the given flag.
static bool fdInUse(int fd, int flag) {
// Determine whether a given fd is already in use with either Read or Write permissions
static bool fdInUse(int fd, Permission perm) {
if (fd < 0 || fd >= SYSCALL_MAXFILES) {
return false;
} else if (fileNames[fd].isEmpty()) {
return false;
} else if ((fileFlags[fd] & flag) ==
static_cast<unsigned>(
flag) /* also compares ie. O_RDONLY (0x0) */) {
return true;
} else if (perm == READ_PERMS) {
return (fileFlags[fd] + 1) & 1; // LSB is 1 if the fd has read perms
}
return false;
return (fileFlags[fd] + 1) & 2; // 2nd LSB is 1 if the fd has write perms
Comment on lines +199 to +201
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mixing arithmetic and bitwise logic is a bit too funky for me. I think these expressions can be rewritten using bit extracts and XORs.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

secondly, we should also extract out the nth LSB is X perms to an enum, e.g. something like

enum FilePermissions {
   RD_PERM_FLAG = 1 << 0,
   WR_PERM_FLAG = 1 << 1
};

}

// Close the file with file descriptor fd. No errors are recoverable -- if
Expand Down Expand Up @@ -289,7 +292,7 @@ class SystemIO : public QObject {
*/
static int seek(int fd, int offset, int base) {
SystemIO::get(); // Ensure that SystemIO is constructed
if (!FileIOData::fdInUse(fd, 0)) // Check the existence of the "read" fd
if (!FileIOData::fdInUse(fd, READ_PERMS)) // Check the existence of the "read" fd
{
s_fileErrorString =
"File descriptor " + QString::number(fd) + " is not open for reading";
Expand Down Expand Up @@ -331,7 +334,7 @@ class SystemIO : public QObject {
/// Read from STDIN file descriptor while using IDE - get input from
/// Messages pane.
if (!FileIOData::fdInUse(fd,
O_RDONLY)) // Check the existence of the "read" fd
READ_PERMS)) // Check the existence of the "read" fd
{
s_fileErrorString =
"File descriptor " + QString::number(fd) + " is not open for reading";
Expand Down Expand Up @@ -406,7 +409,7 @@ class SystemIO : public QObject {
}

if (!FileIOData::fdInUse(
fd, O_WRONLY | O_RDWR)) // Check the existence of the "write" fd
fd, WRITE_PERMS)) // Check the existence of the "write" fd
{
s_fileErrorString =
"File descriptor " + QString::number(fd) + " is not open for writing";
Expand Down
Loading