Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
54 changes: 54 additions & 0 deletions src/libutil-tests/file-descriptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@

#include <cstring>

#ifndef _WIN32
# include <fcntl.h>
# include <stdlib.h>
#endif

namespace nix {

// BufferedSource with configurable small buffer for precise boundary testing.
Expand Down Expand Up @@ -113,6 +118,55 @@ TEST(ReadLine, LineWithNullBytes)
3));
}

#ifndef _WIN32
TEST(ReadLine, TreatsEioAsEof)
{
// Open a pty master. When the slave side is closed (or never opened),
// reading from the master returns EIO, which readLine should treat as EOF.
int master = posix_openpt(O_RDWR | O_NOCTTY);
ASSERT_NE(master, -1);
ASSERT_EQ(grantpt(master), 0);
ASSERT_EQ(unlockpt(master), 0);

// Open and immediately close the slave to trigger EIO on the master.
int slave = open(ptsname(master), O_RDWR | O_NOCTTY);
ASSERT_NE(slave, -1);
close(slave);

// With eofOk=true, readLine should return empty string (treating EIO as EOF).
EXPECT_EQ(readLine(master, /*eofOk=*/true), "");

// With eofOk=false, readLine should throw EndOfFile.
EXPECT_THROW(readLine(master), EndOfFile);

close(master);
}

// macOS (BSD) discards buffered pty data on slave close and returns normal
// EOF (0) instead of EIO, so partial data never reaches the master.
# ifdef __linux__
TEST(ReadLine, PartialLineBeforeEio)
{
int master = posix_openpt(O_RDWR | O_NOCTTY);
ASSERT_NE(master, -1);
ASSERT_EQ(grantpt(master), 0);
ASSERT_EQ(unlockpt(master), 0);

int slave = open(ptsname(master), O_RDWR | O_NOCTTY);
ASSERT_NE(slave, -1);

// Write a partial line (no terminator) from the slave, then close it.
ASSERT_EQ(::write(slave, "partial", 7), 7);
close(slave);

// readLine should return the partial data when eofOk=true.
EXPECT_EQ(readLine(master, /*eofOk=*/true), "partial");

close(master);
}
# endif
#endif

TEST(BufferedSourceReadLine, ReadsLinesFromPipe)
{
Pipe pipe;
Expand Down
14 changes: 12 additions & 2 deletions src/libutil/file-descriptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,18 @@ std::string readLine(Descriptor fd, bool eofOk, char terminator)
checkInterrupt();
char ch;
// FIXME: inefficient
auto rd =
retryOnBlock(fd, PollDirection::In, [&]() { return read(fd, {reinterpret_cast<std::byte *>(&ch), 1}); });
auto rd = retryOnBlock(fd, PollDirection::In, [&]() -> size_t {
try {
return read(fd, {reinterpret_cast<std::byte *>(&ch), 1});
} catch (SystemError & e) {
// On pty masters, EIO signals that the slave side closed,
// which is semantically EOF. Map it to a zero-length read
// so the existing EOF path handles it.
if (e.is(std::errc::io_error))
return 0;
throw;
}
Comment on lines +80 to +89
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we apply the same treatment to FdSource::readLine/readUnbuffered under a certain flag maybe? I don't think we want to keep the one-byte-at-a-time reading function for too long anyway.

});
if (rd == 0) {
if (eofOk)
return s;
Expand Down
Loading