[flang-commits] [flang] ea4758a - [flang] Rework read/write permission management for runtime file opening
peter klausler via flang-commits
flang-commits at lists.llvm.org
Fri Jul 17 14:45:37 PDT 2020
Author: peter klausler
Date: 2020-07-17T14:44:58-07:00
New Revision: ea4758a125298cc25639007509a8012f2f71fb00
URL: https://github.com/llvm/llvm-project/commit/ea4758a125298cc25639007509a8012f2f71fb00
DIFF: https://github.com/llvm/llvm-project/commit/ea4758a125298cc25639007509a8012f2f71fb00.diff
LOG: [flang] Rework read/write permission management for runtime file opening
Anonymous Fortran unit files (e.g., "./fort.7") need to be created
O_RDWR so that they can be written, rewound, and read. Other
files opened with no ACTION= specifier need to set read/write
permissions based on the file, if it exists.
Reviewed By: sscalpone
Differential Revision: https://reviews.llvm.org/D84063
Added:
Modified:
flang/runtime/file.cpp
flang/runtime/file.h
flang/runtime/io-api.cpp
flang/runtime/io-stmt.cpp
flang/runtime/io-stmt.h
flang/runtime/unit.cpp
flang/runtime/unit.h
Removed:
################################################################################
diff --git a/flang/runtime/file.cpp b/flang/runtime/file.cpp
index 19c86a9d4b82..341702df995b 100644
--- a/flang/runtime/file.cpp
+++ b/flang/runtime/file.cpp
@@ -57,63 +57,86 @@ static int openfile_mkstemp(IoErrorHandler &handler) {
return fd;
}
-void OpenFile::Open(
- OpenStatus status, Position position, IoErrorHandler &handler) {
- int flags{mayRead_ ? mayWrite_ ? O_RDWR : O_RDONLY : O_WRONLY};
- switch (status) {
- case OpenStatus::Old:
- if (fd_ >= 0) {
- return;
+void OpenFile::Open(OpenStatus status, std::optional<Action> action,
+ Position position, IoErrorHandler &handler) {
+ if (fd_ >= 0 &&
+ (status == OpenStatus::Old || status == OpenStatus::Unknown)) {
+ return;
+ }
+ if (fd_ >= 0) {
+ if (fd_ <= 2) {
+ // don't actually close a standard file descriptor, we might need it
+ } else {
+ if (::close(fd_) != 0) {
+ handler.SignalErrno();
+ }
}
- knownSize_.reset();
- break;
- case OpenStatus::New:
- flags |= O_CREAT | O_EXCL;
- knownSize_ = 0;
- break;
- case OpenStatus::Scratch:
+ fd_ = -1;
+ }
+ if (status == OpenStatus::Scratch) {
if (path_.get()) {
handler.SignalError("FILE= must not appear with STATUS='SCRATCH'");
path_.reset();
}
+ if (!action) {
+ action = Action::ReadWrite;
+ }
fd_ = openfile_mkstemp(handler);
- knownSize_ = 0;
- return;
- case OpenStatus::Replace:
- flags |= O_CREAT | O_TRUNC;
- knownSize_ = 0;
- break;
- case OpenStatus::Unknown:
- if (fd_ >= 0) {
+ } else {
+ if (!path_.get()) {
+ handler.SignalError(
+ "FILE= is required unless STATUS='OLD' and unit is connected");
return;
}
- flags |= O_CREAT;
- knownSize_.reset();
- break;
- }
- // If we reach this point, we're opening a new file.
- // TODO: Fortran shouldn't create a new file until the first WRITE.
- if (fd_ >= 0) {
- if (fd_ <= 2) {
- // don't actually close a standard file descriptor, we might need it
- } else if (::close(fd_) != 0) {
- handler.SignalErrno();
+ int flags{0};
+ if (status != OpenStatus::Old) {
+ flags |= O_CREAT;
+ }
+ if (status == OpenStatus::New) {
+ flags |= O_EXCL;
+ } else if (status == OpenStatus::Replace) {
+ flags |= O_TRUNC;
+ }
+ if (!action) {
+ // Try to open read/write, back off to read-only on failure
+ fd_ = ::open(path_.get(), flags | O_RDWR, 0600);
+ if (fd_ >= 0) {
+ action = Action::ReadWrite;
+ } else {
+ action = Action::Read;
+ }
+ }
+ if (fd_ < 0) {
+ switch (*action) {
+ case Action::Read:
+ flags |= O_RDONLY;
+ break;
+ case Action::Write:
+ flags |= O_WRONLY;
+ break;
+ case Action::ReadWrite:
+ flags |= O_RDWR;
+ break;
+ }
+ fd_ = ::open(path_.get(), flags, 0600);
+ if (fd_ < 0) {
+ handler.SignalErrno();
+ }
}
}
- if (!path_.get()) {
- handler.SignalError(
- "FILE= is required unless STATUS='OLD' and unit is connected");
- return;
- }
- fd_ = ::open(path_.get(), flags, 0600);
- if (fd_ < 0) {
- handler.SignalErrno();
- }
+ RUNTIME_CHECK(handler, action.has_value());
pending_.reset();
if (position == Position::Append && !RawSeekToEnd()) {
handler.SignalErrno();
}
isTerminal_ = ::isatty(fd_) == 1;
+ mayRead_ = *action != Action::Write;
+ mayWrite_ = *action != Action::Read;
+ if (status == OpenStatus::Old || status == OpenStatus::Unknown) {
+ knownSize_.reset();
+ } else {
+ knownSize_ = 0;
+ }
}
void OpenFile::Predefine(int fd) {
@@ -124,6 +147,9 @@ void OpenFile::Predefine(int fd) {
knownSize_.reset();
nextId_ = 0;
pending_.reset();
+ mayRead_ = fd == 0;
+ mayWrite_ = fd != 0;
+ mayPosition_ = false;
}
void OpenFile::Close(CloseStatus status, IoErrorHandler &handler) {
diff --git a/flang/runtime/file.h b/flang/runtime/file.h
index 17a5e910ecae..1d25a91558a4 100644
--- a/flang/runtime/file.h
+++ b/flang/runtime/file.h
@@ -21,6 +21,7 @@ namespace Fortran::runtime::io {
enum class OpenStatus { Old, New, Scratch, Replace, Unknown };
enum class CloseStatus { Keep, Delete };
enum class Position { AsIs, Rewind, Append };
+enum class Action { Read, Write, ReadWrite };
class OpenFile {
public:
@@ -30,19 +31,16 @@ class OpenFile {
void set_path(OwningPtr<char> &&, std::size_t bytes);
std::size_t pathLength() const { return pathLength_; }
bool mayRead() const { return mayRead_; }
- void set_mayRead(bool yes) { mayRead_ = yes; }
bool mayWrite() const { return mayWrite_; }
- void set_mayWrite(bool yes) { mayWrite_ = yes; }
+ bool mayPosition() const { return mayPosition_; }
bool mayAsynchronous() const { return mayAsynchronous_; }
void set_mayAsynchronous(bool yes) { mayAsynchronous_ = yes; }
- bool mayPosition() const { return mayPosition_; }
- void set_mayPosition(bool yes) { mayPosition_ = yes; }
FileOffset position() const { return position_; }
bool isTerminal() const { return isTerminal_; }
std::optional<FileOffset> knownSize() const { return knownSize_; }
bool IsOpen() const { return fd_ >= 0; }
- void Open(OpenStatus, Position, IoErrorHandler &);
+ void Open(OpenStatus, std::optional<Action>, Position, IoErrorHandler &);
void Predefine(int fd);
void Close(CloseStatus, IoErrorHandler &);
diff --git a/flang/runtime/io-api.cpp b/flang/runtime/io-api.cpp
index 0bd827bc53aa..2f077e1f9ff8 100644
--- a/flang/runtime/io-api.cpp
+++ b/flang/runtime/io-api.cpp
@@ -563,31 +563,31 @@ bool IONAME(SetAction)(Cookie cookie, const char *keyword, std::size_t length) {
io.GetIoErrorHandler().Crash(
"SetAction() called when not in an OPEN statement");
}
- bool mayRead{true};
- bool mayWrite{true};
+ std::optional<Action> action;
static const char *keywords[]{"READ", "WRITE", "READWRITE", nullptr};
switch (IdentifyValue(keyword, length, keywords)) {
case 0:
- mayWrite = false;
+ action = Action::Read;
break;
case 1:
- mayRead = false;
+ action = Action::Write;
break;
case 2:
+ action = Action::ReadWrite;
break;
default:
open->SignalError(IostatErrorInKeyword, "Invalid ACTION='%.*s'",
static_cast<int>(length), keyword);
return false;
}
- if (mayRead != open->unit().mayRead() ||
- mayWrite != open->unit().mayWrite()) {
- if (open->wasExtant()) {
+ RUNTIME_CHECK(io.GetIoErrorHandler(), action.has_value());
+ if (open->wasExtant()) {
+ if ((*action != Action::Write) != open->unit().mayRead() ||
+ (*action != Action::Read) != open->unit().mayWrite()) {
open->SignalError("ACTION= may not be changed on an open unit");
}
- open->unit().set_mayRead(mayRead);
- open->unit().set_mayWrite(mayWrite);
}
+ open->set_action(*action);
return true;
}
diff --git a/flang/runtime/io-stmt.cpp b/flang/runtime/io-stmt.cpp
index 0681da215d1e..70fb3f9350bc 100644
--- a/flang/runtime/io-stmt.cpp
+++ b/flang/runtime/io-stmt.cpp
@@ -165,7 +165,8 @@ int OpenStatementState::EndIoStatement() {
if (wasExtant_ && status_ != OpenStatus::Old) {
SignalError("OPEN statement for connected unit must have STATUS='OLD'");
}
- unit().OpenUnit(status_, position_, std::move(path_), pathLength_, *this);
+ unit().OpenUnit(
+ status_, action_, position_, std::move(path_), pathLength_, *this);
return ExternalIoStatementBase::EndIoStatement();
}
diff --git a/flang/runtime/io-stmt.h b/flang/runtime/io-stmt.h
index 066391bd1566..6f5ca2c48112 100644
--- a/flang/runtime/io-stmt.h
+++ b/flang/runtime/io-stmt.h
@@ -294,15 +294,17 @@ class OpenStatementState : public ExternalIoStatementBase {
: ExternalIoStatementBase{unit, sourceFile, sourceLine}, wasExtant_{
wasExtant} {}
bool wasExtant() const { return wasExtant_; }
- void set_status(OpenStatus status) { status_ = status; }
+ void set_status(OpenStatus status) { status_ = status; } // STATUS=
void set_path(const char *, std::size_t, int kind); // FILE=
void set_position(Position position) { position_ = position; } // POSITION=
+ void set_action(Action action) { action_ = action; } // ACTION=
int EndIoStatement();
private:
bool wasExtant_;
OpenStatus status_{OpenStatus::Unknown};
Position position_{Position::AsIs};
+ std::optional<Action> action_;
OwningPtr<char> path_;
std::size_t pathLength_;
};
diff --git a/flang/runtime/unit.cpp b/flang/runtime/unit.cpp
index cf20d7cd81c6..c6af53e6ec22 100644
--- a/flang/runtime/unit.cpp
+++ b/flang/runtime/unit.cpp
@@ -64,7 +64,8 @@ ExternalFileUnit &ExternalFileUnit::LookUpOrCreateAnonymous(
IoErrorHandler handler{terminator};
result.OpenUnit(
dir == Direction::Input ? OpenStatus::Old : OpenStatus::Replace,
- Position::Rewind, std::move(path), std::strlen(path.get()), handler);
+ Action::ReadWrite, Position::Rewind, std::move(path),
+ std::strlen(path.get()), handler);
result.isUnformatted = isUnformatted;
}
return result;
@@ -87,8 +88,8 @@ int ExternalFileUnit::NewUnit(const Terminator &terminator) {
return GetUnitMap().NewUnit(terminator).unitNumber();
}
-void ExternalFileUnit::OpenUnit(OpenStatus status, Position position,
- OwningPtr<char> &&newPath, std::size_t newPathLength,
+void ExternalFileUnit::OpenUnit(OpenStatus status, std::optional<Action> action,
+ Position position, OwningPtr<char> &&newPath, std::size_t newPathLength,
IoErrorHandler &handler) {
if (IsOpen()) {
if (status == OpenStatus::Old &&
@@ -105,7 +106,7 @@ void ExternalFileUnit::OpenUnit(OpenStatus status, Position position,
Close(CloseStatus::Keep, handler);
}
set_path(std::move(newPath), newPathLength);
- Open(status, position, handler);
+ Open(status, action, position, handler);
auto totalBytes{knownSize()};
if (access == Access::Direct) {
if (!isFixedRecordLength || !recordLength) {
@@ -186,16 +187,10 @@ UnitMap &ExternalFileUnit::GetUnitMap() {
unitMap = New<UnitMap>{terminator}().release();
ExternalFileUnit &out{ExternalFileUnit::CreateNew(6, terminator)};
out.Predefine(1);
- out.set_mayRead(false);
- out.set_mayWrite(true);
- out.set_mayPosition(false);
out.SetDirection(Direction::Output, handler);
defaultOutput = &out;
ExternalFileUnit &in{ExternalFileUnit::CreateNew(5, terminator)};
in.Predefine(0);
- in.set_mayRead(true);
- in.set_mayWrite(false);
- in.set_mayPosition(false);
in.SetDirection(Direction::Input, handler);
defaultInput = ∈
// TODO: Set UTF-8 mode from the environment
diff --git a/flang/runtime/unit.h b/flang/runtime/unit.h
index f0edeedef081..d2d2dce035f1 100644
--- a/flang/runtime/unit.h
+++ b/flang/runtime/unit.h
@@ -48,8 +48,8 @@ class ExternalFileUnit : public ConnectionState,
static void CloseAll(IoErrorHandler &);
static void FlushAll(IoErrorHandler &);
- void OpenUnit(OpenStatus, Position, OwningPtr<char> &&path,
- std::size_t pathLength, IoErrorHandler &);
+ void OpenUnit(OpenStatus, std::optional<Action>, Position,
+ OwningPtr<char> &&path, std::size_t pathLength, IoErrorHandler &);
void CloseUnit(CloseStatus, IoErrorHandler &);
void DestroyClosed();
More information about the flang-commits
mailing list