Skip to content

Commit 166b4d2

Browse files
godlygeekpablogsal
authored andcommitted
Use stdio rather than ifstream
This should both be a bit faster, and makes the error reporting a bit simpler. And incidentally this fixes a file descriptor leak as well. Signed-off-by: Matt Wozniski <[email protected]>
1 parent 4ed6652 commit 166b4d2

File tree

2 files changed

+15
-15
lines changed

2 files changed

+15
-15
lines changed

src/pystack/_pystack/mem.cpp

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
namespace pystack {
1818

1919
using elf_unique_ptr = std::unique_ptr<Elf, std::function<void(Elf*)>>;
20-
using file_unique_ptr = std::unique_ptr<FILE, std::function<int(FILE*)>>;
2120

2221
static ssize_t
2322
_process_vm_readv(
@@ -229,7 +228,7 @@ ProcessMemoryManager::ProcessMemoryManager(pid_t pid)
229228
ssize_t
230229
ProcessMemoryManager::readChunk(remote_addr_t addr, size_t len, char* dst) const
231230
{
232-
if (d_memfile.is_open()) {
231+
if (d_memfile) {
233232
return readChunkThroughMemFile(addr, len, dst);
234233
} else {
235234
return readChunkDirect(addr, len, dst);
@@ -272,25 +271,25 @@ ProcessMemoryManager::readChunkDirect(remote_addr_t addr, size_t len, char* dst)
272271
ssize_t
273272
ProcessMemoryManager::readChunkThroughMemFile(remote_addr_t addr, size_t len, char* dst) const
274273
{
275-
if (!d_memfile.is_open()) {
274+
if (!d_memfile) {
276275
std::string filepath = "/proc/" + std::to_string(d_pid) + "/mem";
277-
d_memfile.open(filepath, std::ifstream::binary);
276+
d_memfile = file_unique_ptr(fopen(filepath.c_str(), "r"), fclose);
278277
if (!d_memfile) {
279-
LOG(ERROR) << "Failed to open file " << filepath;
280-
if (-1 == open(filepath.c_str(), O_RDONLY)) {
281-
if (errno == EPERM || errno == EACCES) {
282-
throw std::runtime_error(PERM_MESSAGE);
283-
}
284-
throw std::system_error(errno, std::generic_category());
278+
if (errno == EPERM || errno == EACCES) {
279+
LOG(ERROR) << "Permission denied opening file " << filepath;
280+
throw std::runtime_error(PERM_MESSAGE);
285281
}
282+
LOG(ERROR) << "Failed to open file " << filepath << ": " << std::strerror(errno);
286283
throw std::runtime_error("Failed to open " + filepath);
287284
}
288285
}
289-
d_memfile.seekg(addr);
290-
if (!d_memfile.read((char*)dst, len)) {
286+
fseeko(d_memfile.get(), addr, SEEK_SET);
287+
if (static_cast<off_t>(addr) != ftello(d_memfile.get())
288+
|| len != fread(dst, 1, len, d_memfile.get()))
289+
{
291290
throw InvalidRemoteAddress();
292291
}
293-
return d_memfile.gcount();
292+
return static_cast<ssize_t>(len);
294293
}
295294

296295
ssize_t

src/pystack/_pystack/mem.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
#include <cstdint>
44
#include <fcntl.h>
5-
#include <fstream>
65
#include <functional>
76
#include <list>
87
#include <memory>
@@ -16,6 +15,8 @@
1615
#include "elf_common.h"
1716

1817
namespace pystack {
18+
19+
using file_unique_ptr = std::unique_ptr<FILE, std::function<int(FILE*)>>;
1920
typedef uintptr_t remote_addr_t;
2021

2122
struct RemoteMemCopyError : public std::exception
@@ -175,7 +176,7 @@ class ProcessMemoryManager : public AbstractRemoteMemoryManager
175176
pid_t d_pid;
176177
std::vector<VirtualMap> d_vmaps;
177178
mutable LRUCache d_lru_cache;
178-
mutable std::ifstream d_memfile{};
179+
mutable file_unique_ptr d_memfile;
179180

180181
// Methods
181182
ssize_t readChunk(remote_addr_t addr, size_t len, char* dst) const;

0 commit comments

Comments
 (0)