Skip to content

Commit

Permalink
Return an empty mapping for an empty file asset (#10815)
Browse files Browse the repository at this point in the history
  • Loading branch information
jason-simmons committed Aug 13, 2019
1 parent 663f9a9 commit 7711efb
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 4 deletions.
2 changes: 1 addition & 1 deletion assets/directory_asset_bundle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ std::unique_ptr<fml::Mapping> DirectoryAssetBundle::GetAsMapping(
auto mapping = std::make_unique<fml::FileMapping>(fml::OpenFile(
descriptor_, asset_name.c_str(), false, fml::FilePermission::kRead));

if (mapping->GetMapping() == nullptr) {
if (!mapping->IsValid()) {
return nullptr;
}

Expand Down
14 changes: 14 additions & 0 deletions fml/file_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,17 @@ TEST(FileTest, AtomicWriteTest) {
// Cleanup.
ASSERT_TRUE(fml::UnlinkFile(dir.fd(), "precious_data"));
}

TEST(FileTest, EmptyMappingTest) {
fml::ScopedTemporaryDirectory dir;

auto file = fml::OpenFile(dir.fd(), "my_contents", true,
fml::FilePermission::kReadWrite);

fml::FileMapping mapping(file);
ASSERT_TRUE(mapping.IsValid());
ASSERT_EQ(mapping.GetSize(), 0ul);
ASSERT_EQ(mapping.GetMapping(), nullptr);

ASSERT_TRUE(fml::UnlinkFile(dir.fd(), "my_contents"));
}
4 changes: 2 additions & 2 deletions fml/mapping.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ std::unique_ptr<FileMapping> FileMapping::CreateReadOnly(
auto mapping = std::make_unique<FileMapping>(
base_fd, std::initializer_list<Protection>{Protection::kRead});

if (mapping->GetSize() == 0 || mapping->GetMapping() == nullptr) {
if (!mapping->IsValid()) {
return nullptr;
}

Expand All @@ -56,7 +56,7 @@ std::unique_ptr<FileMapping> FileMapping::CreateReadExecute(
base_fd, std::initializer_list<Protection>{Protection::kRead,
Protection::kExecute});

if (mapping->GetSize() == 0 || mapping->GetMapping() == nullptr) {
if (!mapping->IsValid()) {
return nullptr;
}

Expand Down
3 changes: 3 additions & 0 deletions fml/mapping.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ class FileMapping final : public Mapping {

uint8_t* GetMutableMapping();

bool IsValid() const;

private:
bool valid_ = false;
size_t size_ = 0;
uint8_t* mapping_ = nullptr;
uint8_t* mutable_mapping_ = nullptr;
Expand Down
8 changes: 7 additions & 1 deletion fml/platform/posix/mapping_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ FileMapping::FileMapping(const fml::UniqueFD& handle,
return;
}

if (stat_buffer.st_size <= 0) {
if (stat_buffer.st_size == 0) {
valid_ = true;
return;
}

Expand All @@ -79,6 +80,7 @@ FileMapping::FileMapping(const fml::UniqueFD& handle,

mapping_ = static_cast<uint8_t*>(mapping);
size_ = stat_buffer.st_size;
valid_ = true;
if (is_writable) {
mutable_mapping_ = mapping_;
}
Expand All @@ -98,4 +100,8 @@ const uint8_t* FileMapping::GetMapping() const {
return mapping_;
}

bool FileMapping::IsValid() const {
return valid_;
}

} // namespace fml
10 changes: 10 additions & 0 deletions fml/platform/win/mapping_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ FileMapping::FileMapping(const fml::UniqueFD& fd,
return;
}

if (mapping_size == 0) {
valid_ = true;
return;
}

DWORD protect_flags = 0;
bool read_only = !IsWritable(protections);

Expand Down Expand Up @@ -90,6 +95,7 @@ FileMapping::FileMapping(const fml::UniqueFD& fd,

mapping_ = mapping;
size_ = mapping_size;
valid_ = true;
if (IsWritable(protections)) {
mutable_mapping_ = mapping_;
}
Expand All @@ -109,4 +115,8 @@ const uint8_t* FileMapping::GetMapping() const {
return mapping_;
}

bool FileMapping::IsValid() const {
return valid_;
}

} // namespace fml

0 comments on commit 7711efb

Please sign in to comment.