Remove fd_set_t

Now that we no longer need to worry about pipes conflicting with
user-specified redirections, we can remove fd_set_t.
This commit is contained in:
ridiculousfish 2021-02-05 18:14:50 -08:00
parent b79ec0122a
commit b5716e97cc
6 changed files with 14 additions and 78 deletions

View file

@ -38,8 +38,7 @@ maybe_t<int> builtin_eval(parser_t &parser, io_streams_t &streams, wchar_t **arg
// buffer in that case.
shared_ptr<io_bufferfill_t> stdout_fill{};
if (streams.out_is_piped) {
stdout_fill =
io_bufferfill_t::create(fd_set_t{}, parser.libdata().read_limit, STDOUT_FILENO);
stdout_fill = io_bufferfill_t::create(parser.libdata().read_limit, STDOUT_FILENO);
if (!stdout_fill) {
// We were unable to create a pipe, probably fd exhaustion.
return STATUS_CMD_ERROR;
@ -50,8 +49,7 @@ maybe_t<int> builtin_eval(parser_t &parser, io_streams_t &streams, wchar_t **arg
// Of course the same applies to stderr.
shared_ptr<io_bufferfill_t> stderr_fill{};
if (streams.err_is_piped) {
stderr_fill =
io_bufferfill_t::create(fd_set_t{}, parser.libdata().read_limit, STDERR_FILENO);
stderr_fill = io_bufferfill_t::create(parser.libdata().read_limit, STDERR_FILENO);
if (!stderr_fill) {
return STATUS_CMD_ERROR;
}

View file

@ -658,16 +658,15 @@ static proc_performer_t get_performer_for_process(process_t *p, job_t *job,
}
/// Execute a block node or function "process".
/// \p conflicts contains the list of fds which pipes should avoid.
/// \p allow_buffering if true, permit buffering the output.
static launch_result_t exec_block_or_func_process(parser_t &parser, const std::shared_ptr<job_t> &j,
process_t *p, const fd_set_t &conflicts,
io_chain_t io_chain, bool allow_buffering) {
process_t *p, io_chain_t io_chain,
bool allow_buffering) {
// Create an output buffer if we're piping to another process.
shared_ptr<io_bufferfill_t> block_output_bufferfill{};
if (!p->is_last_in_job && allow_buffering) {
// Be careful to handle failure, e.g. too many open fds.
block_output_bufferfill = io_bufferfill_t::create(conflicts);
block_output_bufferfill = io_bufferfill_t::create();
if (!block_output_bufferfill) {
return launch_result_t::failed;
}
@ -709,7 +708,6 @@ static launch_result_t exec_block_or_func_process(parser_t &parser, const std::s
static launch_result_t exec_process_in_job(parser_t &parser, process_t *p,
const std::shared_ptr<job_t> &j,
const io_chain_t &block_io, autoclose_pipes_t pipes,
const fd_set_t &conflicts,
const autoclose_pipes_t &deferred_pipes,
bool is_deferred_run = false) {
// The write pipe (destined for stdout) needs to occur before redirections. For example,
@ -800,8 +798,8 @@ static launch_result_t exec_process_in_job(parser_t &parser, process_t *p,
// Allow buffering unless this is a deferred run. If deferred, then processes after us
// were already launched, so they are ready to receive (or reject) our output.
bool allow_buffering = !is_deferred_run;
if (exec_block_or_func_process(parser, j, p, conflicts, process_net_io_chain,
allow_buffering) == launch_result_t::failed) {
if (exec_block_or_func_process(parser, j, p, process_net_io_chain, allow_buffering) ==
launch_result_t::failed) {
return launch_result_t::failed;
}
break;
@ -907,14 +905,6 @@ bool exec_job(parser_t &parser, const shared_ptr<job_t> &j, const io_chain_t &bl
return true;
}
// Get the list of all FDs so we can ensure our pipes do not conflict.
fd_set_t conflicts = block_io.fd_set();
for (const auto &p : j->processes) {
for (const auto &spec : p->redirection_specs()) {
conflicts.add(spec.fd);
}
}
// Handle an exec call.
if (j->processes.front()->type == process_type_t::exec) {
// If we are interactive, perhaps disallow exec if there are background jobs.
@ -987,8 +977,8 @@ bool exec_job(parser_t &parser, const shared_ptr<job_t> &j, const io_chain_t &bl
}
// Regular process.
if (exec_process_in_job(parser, p, j, block_io, std::move(proc_pipes), conflicts,
deferred_pipes) == launch_result_t::failed) {
if (exec_process_in_job(parser, p, j, block_io, std::move(proc_pipes), deferred_pipes) ==
launch_result_t::failed) {
aborted_pipeline = true;
abort_pipeline_from(j, p);
break;
@ -1012,7 +1002,7 @@ bool exec_job(parser_t &parser, const shared_ptr<job_t> &j, const io_chain_t &bl
// Some other process already aborted our pipeline.
deferred_process->mark_aborted_before_launch();
} else if (exec_process_in_job(parser, deferred_process, j, block_io,
std::move(deferred_pipes), conflicts, {},
std::move(deferred_pipes), {},
true) == launch_result_t::failed) {
// The deferred proc itself failed to launch.
deferred_process->mark_aborted_before_launch();
@ -1104,7 +1094,7 @@ static int exec_subshell_internal(const wcstring &cmd, parser_t &parser,
// IO buffer creation may fail (e.g. if we have too many open files to make a pipe), so this may
// be null.
auto bufferfill = io_bufferfill_t::create(fd_set_t{}, ld.read_limit);
auto bufferfill = io_bufferfill_t::create(ld.read_limit);
if (!bufferfill) {
*break_expand = true;
return STATUS_CMD_ERROR;

View file

@ -71,26 +71,6 @@ struct autoclose_pipes_t {
: read(std::move(r)), write(std::move(w)) {}
};
/// A simple set of FDs.
struct fd_set_t {
std::vector<bool> fds;
void add(int fd) {
assert(fd >= 0 && "Invalid fd");
if (static_cast<size_t>(fd) >= fds.size()) {
fds.resize(fd + 1);
}
fds[fd] = true;
}
bool contains(int fd) const {
assert(fd >= 0 && "Invalid fd");
return static_cast<size_t>(fd) < fds.size() && fds[fd];
}
bool empty() const { return fds.empty(); }
};
/// Call pipe(), populating autoclose fds.
/// The pipes are marked CLO_EXEC and are placed in the high fd range.
/// \return pipes on success, none() on error.

View file

@ -1293,7 +1293,7 @@ static void test_parser() {
}
static void test_1_cancellation(const wchar_t *src) {
auto filler = io_bufferfill_t::create(fd_set_t{});
auto filler = io_bufferfill_t::create();
pthread_t thread = pthread_self();
double delay = 0.50 /* seconds */;
iothread_perform([=]() {
@ -3541,21 +3541,6 @@ static void test_input() {
}
}
static void test_fd_set() {
say(L"Testing fd_set");
fd_set_t fds;
do_test(!fds.contains(0));
do_test(!fds.contains(100));
fds.add(1);
do_test(!fds.contains(0));
do_test(!fds.contains(100));
do_test(fds.contains(1));
fds.add(1);
do_test(!fds.contains(0));
do_test(!fds.contains(100));
do_test(fds.contains(1));
}
static void test_line_iterator() {
say(L"Testing line iterator");
@ -6335,7 +6320,6 @@ int main(int argc, char **argv) {
if (should_test_function("complete")) test_complete();
if (should_test_function("autoload")) test_autoload();
if (should_test_function("input")) test_input();
if (should_test_function("io")) test_fd_set();
if (should_test_function("line_iterator")) test_line_iterator();
if (should_test_function("undo")) test_undo();
if (should_test_function("universal")) test_universal();

View file

@ -156,9 +156,7 @@ separated_buffer_t io_buffer_t::complete_background_fillthread_and_take_buffer()
return result;
}
shared_ptr<io_bufferfill_t> io_bufferfill_t::create(const fd_set_t &conflicts, size_t buffer_limit,
int target) {
(void)conflicts;
shared_ptr<io_bufferfill_t> io_bufferfill_t::create(size_t buffer_limit, int target) {
assert(target >= 0 && "Invalid target fd");
// Construct our pipes.
@ -277,14 +275,6 @@ void io_chain_t::print() const {
}
}
fd_set_t io_chain_t::fd_set() const {
fd_set_t result;
for (const auto &io : *this) {
result.add(io->fd);
}
return result;
}
shared_ptr<const io_data_t> io_chain_t::io_for_fd(int fd) const {
for (auto iter = rbegin(); iter != rend(); ++iter) {
const auto &data = *iter;

View file

@ -276,10 +276,7 @@ class io_bufferfill_t final : public io_data_t {
/// \returns nullptr on failure, e.g. too many open fds.
///
/// \param target the fd which this will be dup2'd to - typically stdout.
/// \param conflicts A set of fds. The function ensures that any pipe it makes does
/// not conflict with an fd redirection in this list.
static shared_ptr<io_bufferfill_t> create(const fd_set_t &conflicts, size_t buffer_limit = 0,
int target = STDOUT_FILENO);
static shared_ptr<io_bufferfill_t> create(size_t buffer_limit = 0, int target = STDOUT_FILENO);
/// Reset the receiver (possibly closing the write end of the pipe), and complete the fillthread
/// of the buffer. \return the buffer.
@ -357,9 +354,6 @@ class io_chain_t : public std::vector<io_data_ref_t> {
/// Output debugging information to stderr.
void print() const;
/// \return the set of redirected FDs.
fd_set_t fd_set() const;
};
/// Base class representing the output that a builtin can generate.