Merge pull request #132 from mingnus/master
Fix potential file descriptor leak and compile flags
This commit is contained in:
commit
70f11e0866
@ -39,14 +39,16 @@ namespace {
|
||||
|
||||
//----------------------------------------------------------------
|
||||
|
||||
int
|
||||
file_utils::open_file(string const &path, int flags) {
|
||||
int fd = ::open(path.c_str(), OPEN_FLAGS | flags, DEFAULT_MODE);
|
||||
if (fd < 0)
|
||||
file_utils::file_descriptor::file_descriptor(string const &path, int flags) {
|
||||
fd_ = ::open(path.c_str(), OPEN_FLAGS | flags, DEFAULT_MODE);
|
||||
if (fd_ < 0)
|
||||
syscall_failed("open",
|
||||
"Note: you cannot run this tool with these options on live metadata.");
|
||||
}
|
||||
|
||||
return fd;
|
||||
file_utils::file_descriptor::~file_descriptor() {
|
||||
close(fd_);
|
||||
fd_ = -1;
|
||||
}
|
||||
|
||||
bool
|
||||
@ -76,7 +78,7 @@ file_utils::check_file_exists(string const &file, bool must_be_regular_file) {
|
||||
throw runtime_error("Not a regular file");
|
||||
}
|
||||
|
||||
int
|
||||
file_utils::file_descriptor
|
||||
file_utils::create_block_file(string const &path, off_t file_size) {
|
||||
if (file_exists(path)) {
|
||||
ostringstream out;
|
||||
@ -84,16 +86,16 @@ file_utils::create_block_file(string const &path, off_t file_size) {
|
||||
throw runtime_error(out.str());
|
||||
}
|
||||
|
||||
int fd = open_file(path, O_CREAT | O_EXCL | O_RDWR);
|
||||
file_descriptor fd(path, O_CREAT | O_EXCL | O_RDWR);
|
||||
|
||||
int r = ::ftruncate(fd, file_size);
|
||||
int r = ::ftruncate(fd.fd_, file_size);
|
||||
if (r < 0)
|
||||
syscall_failed("ftruncate");
|
||||
|
||||
return fd;
|
||||
}
|
||||
|
||||
int
|
||||
file_utils::file_descriptor
|
||||
file_utils::open_block_file(string const &path, off_t min_size, bool writeable, bool excl) {
|
||||
if (!file_exists(path)) {
|
||||
ostringstream out;
|
||||
@ -105,7 +107,7 @@ file_utils::open_block_file(string const &path, off_t min_size, bool writeable,
|
||||
if (excl)
|
||||
flags |= O_EXCL;
|
||||
|
||||
return open_file(path, flags);
|
||||
return file_descriptor(path, flags);
|
||||
}
|
||||
|
||||
uint64_t
|
||||
@ -146,17 +148,15 @@ file_utils::zero_superblock(std::string const &path)
|
||||
{
|
||||
char *buffer;
|
||||
unsigned const SUPERBLOCK_SIZE = 4096;
|
||||
int fd = open_block_file(path, SUPERBLOCK_SIZE, true, true);
|
||||
file_descriptor fd = open_block_file(path, SUPERBLOCK_SIZE, true, true);
|
||||
|
||||
buffer = reinterpret_cast<char *>(aligned_alloc(SUPERBLOCK_SIZE, SUPERBLOCK_SIZE));
|
||||
if (!buffer)
|
||||
throw runtime_error("out of memory");
|
||||
|
||||
memset(buffer, 0, SUPERBLOCK_SIZE);
|
||||
if (::write(fd, buffer, SUPERBLOCK_SIZE) != SUPERBLOCK_SIZE)
|
||||
if (::write(fd.fd_, buffer, SUPERBLOCK_SIZE) != SUPERBLOCK_SIZE)
|
||||
throw runtime_error("couldn't zero superblock");
|
||||
|
||||
::close(fd);
|
||||
}
|
||||
|
||||
//----------------------------------------------------------------
|
||||
|
@ -8,11 +8,20 @@
|
||||
//----------------------------------------------------------------
|
||||
|
||||
namespace file_utils {
|
||||
int open_file(std::string const &path, int flags);
|
||||
struct file_descriptor {
|
||||
// file_descriptor is movable but not copyable
|
||||
file_descriptor(file_descriptor &&) = default;
|
||||
file_descriptor& operator=(file_descriptor &&) = default;
|
||||
file_descriptor(std::string const &path, int flags);
|
||||
virtual ~file_descriptor();
|
||||
|
||||
int fd_;
|
||||
};
|
||||
|
||||
bool file_exists(std::string const &path);
|
||||
void check_file_exists(std::string const &file, bool must_be_regular_file = true);
|
||||
int create_block_file(std::string const &path, off_t file_size);
|
||||
int open_block_file(std::string const &path, off_t min_size, bool writeable, bool excl = true);
|
||||
file_descriptor create_block_file(std::string const &path, off_t file_size);
|
||||
file_descriptor open_block_file(std::string const &path, off_t min_size, bool writeable, bool excl = true);
|
||||
uint64_t get_file_length(std::string const &file);
|
||||
void zero_superblock(std::string const &path);
|
||||
}
|
||||
|
@ -15,6 +15,7 @@
|
||||
#include <sstream>
|
||||
|
||||
using namespace bcache;
|
||||
using namespace file_utils;
|
||||
|
||||
//----------------------------------------------------------------
|
||||
|
||||
@ -290,7 +291,7 @@ block_cache::setup_control_block(block &b)
|
||||
size_t block_size_bytes = block_size_ << SECTOR_SHIFT;
|
||||
|
||||
memset(cb, 0, sizeof(*cb));
|
||||
cb->aio_fildes = fd_;
|
||||
cb->aio_fildes = fd_.fd_;
|
||||
|
||||
cb->u.c.buf = b.data_;
|
||||
cb->u.c.offset = block_size_bytes * b.index_;
|
||||
@ -371,8 +372,9 @@ block_cache::calc_nr_buckets(unsigned nr_blocks)
|
||||
return r;
|
||||
}
|
||||
|
||||
block_cache::block_cache(int fd, sector_t block_size, uint64_t on_disk_blocks, size_t mem)
|
||||
: nr_locked_(0),
|
||||
block_cache::block_cache(file_descriptor &fd, sector_t block_size, uint64_t on_disk_blocks, size_t mem)
|
||||
: fd_(fd),
|
||||
nr_locked_(0),
|
||||
nr_dirty_(0),
|
||||
nr_io_pending_(0),
|
||||
read_hits_(0),
|
||||
@ -386,7 +388,6 @@ block_cache::block_cache(int fd, sector_t block_size, uint64_t on_disk_blocks, s
|
||||
int r;
|
||||
unsigned nr_cache_blocks = calc_nr_cache_blocks(mem, block_size);
|
||||
|
||||
fd_ = fd;
|
||||
block_size_ = block_size;
|
||||
nr_data_blocks_ = on_disk_blocks;
|
||||
nr_cache_blocks_ = nr_cache_blocks;
|
||||
@ -418,8 +419,6 @@ block_cache::~block_cache()
|
||||
if (aio_context_)
|
||||
io_destroy(aio_context_);
|
||||
|
||||
::close(fd_);
|
||||
|
||||
#if 0
|
||||
std::cerr << "\nblock cache stats\n"
|
||||
<< "=================\n"
|
||||
|
@ -2,6 +2,7 @@
|
||||
#define BLOCK_CACHE_H
|
||||
|
||||
#include "base/container_of.h"
|
||||
#include "base/file_utils.h"
|
||||
|
||||
#include <boost/intrusive/list.hpp>
|
||||
#include <boost/intrusive/set.hpp>
|
||||
@ -185,7 +186,7 @@ namespace bcache {
|
||||
|
||||
//--------------------------------
|
||||
|
||||
block_cache(int fd, sector_t block_size,
|
||||
block_cache(file_utils::file_descriptor &fd, sector_t block_size,
|
||||
uint64_t max_nr_blocks, size_t mem);
|
||||
~block_cache();
|
||||
|
||||
@ -247,7 +248,7 @@ namespace bcache {
|
||||
|
||||
//--------------------------------
|
||||
|
||||
int fd_;
|
||||
file_utils::file_descriptor &fd_;
|
||||
sector_t block_size_;
|
||||
uint64_t nr_data_blocks_;
|
||||
uint64_t nr_cache_blocks_;
|
||||
|
@ -136,11 +136,12 @@ namespace persistent_data {
|
||||
|
||||
private:
|
||||
uint64_t choose_cache_size(block_address nr_blocks) const;
|
||||
int open_or_create_block_file(std::string const &path, off_t file_size,
|
||||
mode m, bool excl);
|
||||
file_utils::file_descriptor open_or_create_block_file(std::string const &path,
|
||||
off_t file_size,
|
||||
mode m, bool excl);
|
||||
void check(block_address b) const;
|
||||
|
||||
int fd_;
|
||||
file_utils::file_descriptor fd_;
|
||||
mutable block_cache bc_;
|
||||
unsigned superblock_ref_count_;
|
||||
};
|
||||
|
@ -154,7 +154,7 @@ namespace persistent_data {
|
||||
}
|
||||
|
||||
template <uint32_t BlockSize>
|
||||
int
|
||||
file_utils::file_descriptor
|
||||
block_manager<BlockSize>::open_or_create_block_file(std::string const &path, off_t file_size, mode m, bool excl)
|
||||
{
|
||||
switch (m) {
|
||||
|
@ -18,7 +18,7 @@ cache_stream::cache_stream(string const &path,
|
||||
|
||||
// hack because cache uses LRU rather than MRU
|
||||
cache_blocks_((cache_mem / block_size) / 2u),
|
||||
fd_(file_utils::open_file(path, O_RDONLY | O_EXCL)),
|
||||
fd_(path, O_RDONLY | O_EXCL),
|
||||
v_(new bcache::noop_validator()),
|
||||
cache_(new block_cache(fd_, block_size / 512, nr_blocks_, cache_mem)),
|
||||
current_index_(0) {
|
||||
|
@ -38,7 +38,7 @@ namespace thin_provisioning {
|
||||
block_address nr_blocks_;
|
||||
block_address cache_blocks_;
|
||||
|
||||
int fd_;
|
||||
file_utils::file_descriptor fd_;
|
||||
validator::ptr v_;
|
||||
std::unique_ptr<block_cache> cache_;
|
||||
|
||||
|
@ -36,9 +36,9 @@ GMOCK_DEPS=\
|
||||
lib/libgmock.a: $(GMOCK_DEPS)
|
||||
@echo " [CXX] gtest"
|
||||
@mkdir -p lib
|
||||
$(V)g++ $(GMOCK_INCLUDES) -I$(GMOCK_DIR)/googletest -c $(GMOCK_DIR)/googletest/src/gtest-all.cc
|
||||
$(V)g++ $(GMOCK_INCLUDES) -I$(GMOCK_DIR)/googletest -std=c++11 -c $(GMOCK_DIR)/googletest/src/gtest-all.cc
|
||||
@echo " [CXX] gmock"
|
||||
$(V)g++ $(GMOCK_INCLUDES) -I$(GMOCK_DIR)/googlemock -c $(GMOCK_DIR)/googlemock/src/gmock-all.cc
|
||||
$(V)g++ $(GMOCK_INCLUDES) -I$(GMOCK_DIR)/googlemock -std=c++11 -c $(GMOCK_DIR)/googlemock/src/gmock-all.cc
|
||||
@echo " [AR] $<"
|
||||
$(V)ar -rv lib/libgmock.a gtest-all.o gmock-all.o > /dev/null 2>&1
|
||||
|
||||
|
@ -38,7 +38,7 @@ TEST(BCacheTests, cleaned_on_demand)
|
||||
unsigned const NR_BLOCKS = 16;
|
||||
|
||||
temp_file tmp("bcache_t", 1);
|
||||
int fd = open(tmp.get_path().c_str(), O_RDWR | O_DIRECT, 0666);
|
||||
file_utils::file_descriptor fd(tmp.get_path().c_str(), O_RDWR | O_DIRECT);
|
||||
|
||||
uint64_t bs = 8;
|
||||
block_cache bc(fd, bs, 64, (bs << SECTOR_SHIFT) * NR_BLOCKS);
|
||||
|
Loading…
Reference in New Issue
Block a user