Skip to content

Commit 25ef03a

Browse files
committed
fix(vfs/posix): various vfs fixes for Linux
While trying to find what could possibly cause files on disk to become corrupted on Linux I started going through the VFS implementation for Linux and noticed a few issues. This fixes a few issues here: Read/ReadBulk/Write don't try to loop until all of their bytes are written RenameFile returns `0` on success, while we were checking if it was successful by checking if it was `!= 0` GetLength was still reading the buf even though there isn't valid data in it. See: https://man7.org/linux/man-pages/man2/write.2.html https://man7.org/linux/man-pages/man2/read.2.html https://man7.org/linux/man-pages/man2/pread.2.html https://man7.org/linux/man-pages/man3/fstat.3p.html https://man7.org/linux/man-pages/man2/rename.2.html
1 parent 2e87af4 commit 25ef03a

1 file changed

Lines changed: 83 additions & 8 deletions

File tree

code/components/vfs-impl-server/src/PlatformDevice.Posix.cpp

Lines changed: 83 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,28 +62,100 @@ Device::THandle LocalDevice::Create(const std::string& filename, bool createIfEx
6262
size_t LocalDevice::Read(THandle handle, void* outBuffer, size_t size)
6363
{
6464
assert(handle != Device::InvalidHandle);
65+
uint8_t* outByteBuffer = static_cast<uint8_t*>(outBuffer);
6566

66-
ssize_t bytesRead = read(static_cast<int>(handle), outBuffer, size);
67+
size_t totalBytesRead = 0;
6768

68-
return (bytesRead < 0) ? -1 : static_cast<size_t>(bytesRead);
69+
while (totalBytesRead < size)
70+
{
71+
ssize_t bytesRead = read(static_cast<int>(handle), outByteBuffer + totalBytesRead, size - totalBytesRead);
72+
if (bytesRead < 0)
73+
{
74+
// we got interrupted, we should retry our read
75+
if (errno == EINTR)
76+
{
77+
continue;
78+
}
79+
80+
// if we get any error we should treat it as a failure
81+
return -1;
82+
}
83+
84+
if (bytesRead == 0)
85+
{
86+
break;
87+
}
88+
89+
totalBytesRead += static_cast<size_t>(bytesRead);
90+
}
91+
92+
return totalBytesRead;
6993
}
7094

7195
size_t LocalDevice::ReadBulk(THandle handle, uint64_t ptr, void* outBuffer, size_t size)
7296
{
7397
assert(handle != Device::InvalidHandle);
7498

75-
ssize_t bytesRead = pread(static_cast<int>(handle), outBuffer, size, static_cast<off_t>(ptr));
99+
uint8_t* outByteBuffer = static_cast<uint8_t*>(outBuffer);
100+
101+
size_t totalBytesRead = 0;
102+
103+
while (totalBytesRead < size)
104+
{
105+
ssize_t bytesRead = pread(static_cast<int>(handle), outByteBuffer + totalBytesRead, size - totalBytesRead, static_cast<off_t>(ptr + totalBytesRead));
106+
if (bytesRead < 0)
107+
{
108+
// we got interrupted, we should retry our pread
109+
if (errno == EINTR)
110+
{
111+
continue;
112+
}
113+
114+
// if we get any errors we should treat it as a failure
115+
return -1;
116+
}
76117

77-
return (bytesRead < 0) ? -1 : static_cast<size_t>(bytesRead);
118+
if (bytesRead == 0)
119+
{
120+
break;
121+
}
122+
123+
totalBytesRead += static_cast<size_t>(bytesRead);
124+
}
125+
126+
return totalBytesRead;
78127
}
79128

80129
size_t LocalDevice::Write(THandle handle, const void* buffer, size_t size)
81130
{
82131
assert(handle != Device::InvalidHandle);
132+
const uint8_t* writeBuffer = static_cast<const uint8_t*>(buffer);
83133

84-
ssize_t bytesWritten = write(static_cast<int>(handle), buffer, size);
134+
size_t totalBytesWritten = 0;
135+
136+
while (totalBytesWritten < size)
137+
{
138+
ssize_t bytesWritten = write(static_cast<int>(handle), writeBuffer + totalBytesWritten, size - totalBytesWritten);
139+
if (bytesWritten < 0)
140+
{
141+
// if we got interrupted we should retry our write
142+
if (errno == EINTR)
143+
{
144+
continue;
145+
}
85146

86-
return (bytesWritten < 0) ? -1 : static_cast<size_t>(bytesWritten);
147+
return -1;
148+
}
149+
150+
if (bytesWritten == 0)
151+
{
152+
break;
153+
}
154+
155+
totalBytesWritten += static_cast<size_t>(bytesWritten);
156+
}
157+
158+
return totalBytesWritten;
87159
}
88160

89161
size_t LocalDevice::WriteBulk(THandle handle, uint64_t ptr, const void* buffer, size_t size)
@@ -117,7 +189,7 @@ bool LocalDevice::RemoveFile(const std::string& filename)
117189

118190
bool LocalDevice::RenameFile(const std::string& from, const std::string& to)
119191
{
120-
return (rename(from.c_str(), to.c_str()) != 0);
192+
return (rename(from.c_str(), to.c_str()) == 0);
121193
}
122194

123195
bool LocalDevice::CreateDirectory(const std::string& name)
@@ -144,7 +216,10 @@ std::time_t LocalDevice::GetModifiedTime(const std::string& fileName)
144216
size_t LocalDevice::GetLength(THandle handle)
145217
{
146218
struct stat buf;
147-
fstat(static_cast<int>(handle), &buf);
219+
if (fstat(static_cast<int>(handle), &buf) < 0)
220+
{
221+
return 0;
222+
}
148223

149224
return buf.st_size;
150225
}

0 commit comments

Comments
 (0)