Skip to content

Commit

Permalink
Fix code smells reported by chrome's clang plugin (flutter#6833)
Browse files Browse the repository at this point in the history
  • Loading branch information
goderbauer authored Nov 13, 2018
1 parent 4959b71 commit 09ef73f
Show file tree
Hide file tree
Showing 80 changed files with 491 additions and 188 deletions.
2 changes: 1 addition & 1 deletion assets/asset_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class AssetManager final : public AssetResolver {
public:
AssetManager();

~AssetManager();
~AssetManager() override;

void PushFront(std::unique_ptr<AssetResolver> resolver);

Expand Down
4 changes: 4 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ FILE: ../../../flutter/flow/compositor_context.cc
FILE: ../../../flutter/flow/compositor_context.h
FILE: ../../../flutter/flow/debug_print.cc
FILE: ../../../flutter/flow/debug_print.h
FILE: ../../../flutter/flow/embedded_views.cc
FILE: ../../../flutter/flow/embedded_views.h
FILE: ../../../flutter/flow/export_node.cc
FILE: ../../../flutter/flow/export_node.h
Expand Down Expand Up @@ -189,6 +190,7 @@ FILE: ../../../flutter/fml/task_runner.cc
FILE: ../../../flutter/fml/task_runner.h
FILE: ../../../flutter/fml/thread.cc
FILE: ../../../flutter/fml/thread.h
FILE: ../../../flutter/fml/thread_local.cc
FILE: ../../../flutter/fml/thread_local.h
FILE: ../../../flutter/fml/thread_local_unittests.cc
FILE: ../../../flutter/fml/thread_unittests.cc
Expand Down Expand Up @@ -311,6 +313,7 @@ FILE: ../../../flutter/lib/ui/window/pointer_data.cc
FILE: ../../../flutter/lib/ui/window/pointer_data.h
FILE: ../../../flutter/lib/ui/window/pointer_data_packet.cc
FILE: ../../../flutter/lib/ui/window/pointer_data_packet.h
FILE: ../../../flutter/lib/ui/window/viewport_metrics.cc
FILE: ../../../flutter/lib/ui/window/viewport_metrics.h
FILE: ../../../flutter/lib/ui/window/window.cc
FILE: ../../../flutter/lib/ui/window/window.h
Expand Down Expand Up @@ -496,6 +499,7 @@ FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatfor
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPluginAppLifeCycleDelegate.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterStandardCodec.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterStandardCodec_Internal.h
Expand Down
6 changes: 6 additions & 0 deletions common/settings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@

namespace blink {

Settings::Settings() = default;

Settings::Settings(const Settings& other) = default;

Settings::~Settings() = default;

std::string Settings::ToString() const {
std::stringstream stream;
stream << "Settings: " << std::endl;
Expand Down
6 changes: 6 additions & 0 deletions common/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ using TaskObserverAdd =
using TaskObserverRemove = std::function<void(intptr_t /* key */)>;

struct Settings {
Settings();

Settings(const Settings& other);

~Settings();

// VM settings
std::string vm_snapshot_data_path;
std::string vm_snapshot_instr_path;
Expand Down
2 changes: 2 additions & 0 deletions common/task_runners.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ TaskRunners::TaskRunners(std::string label,
ui_(std::move(ui)),
io_(std::move(io)) {}

TaskRunners::TaskRunners(const TaskRunners& other) = default;

TaskRunners::~TaskRunners() = default;

const std::string& TaskRunners::GetLabel() const {
Expand Down
2 changes: 2 additions & 0 deletions common/task_runners.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ class TaskRunners {
fml::RefPtr<fml::TaskRunner> ui,
fml::RefPtr<fml::TaskRunner> io);

TaskRunners(const TaskRunners& other);

~TaskRunners();

const std::string& GetLabel() const;
Expand Down
1 change: 1 addition & 0 deletions flow/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ source_set("flow") {
"compositor_context.h",
"debug_print.cc",
"debug_print.h",
"embedded_views.cc",
"embedded_views.h",
"instrumentation.cc",
"instrumentation.h",
Expand Down
12 changes: 12 additions & 0 deletions flow/embedded_views.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "flutter/flow/embedded_views.h"

namespace flow {

bool ExternalViewEmbedder::SubmitFrame(GrContext* context) {
return false;
};
} // namespace flow
2 changes: 1 addition & 1 deletion flow/embedded_views.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class ExternalViewEmbedder {
virtual SkCanvas* CompositeEmbeddedView(int view_id,
const EmbeddedViewParams& params) = 0;

virtual bool SubmitFrame(GrContext* context) { return false; };
virtual bool SubmitFrame(GrContext* context);

virtual ~ExternalViewEmbedder() = default;

Expand Down
2 changes: 1 addition & 1 deletion flow/layers/layer_tree.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ sk_sp<SkPicture> LayerTree::Flatten(const SkRect& bounds) {
TRACE_EVENT0("flutter", "LayerTree::Flatten");

SkPictureRecorder recorder;
auto canvas = recorder.beginRecording(bounds);
auto* canvas = recorder.beginRecording(bounds);

if (!canvas) {
return nullptr;
Expand Down
2 changes: 1 addition & 1 deletion flow/layers/picture_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ PictureLayer::~PictureLayer() = default;
void PictureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
SkPicture* sk_picture = picture();

if (auto cache = context->raster_cache) {
if (auto* cache = context->raster_cache) {
SkMatrix ctm = matrix;
ctm.postTranslate(offset_.x(), offset_.y());
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
Expand Down
10 changes: 10 additions & 0 deletions flow/raster_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@

namespace flow {

RasterCacheResult::RasterCacheResult() {}

RasterCacheResult::RasterCacheResult(const RasterCacheResult& other) = default;

RasterCacheResult::~RasterCacheResult() = default;

RasterCacheResult::RasterCacheResult(sk_sp<SkImage> image,
const SkRect& logical_rect)
: image_(std::move(image)), logical_rect_(logical_rect) {}

void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const {
SkAutoCanvasRestore auto_restore(&canvas, true);
SkIRect bounds =
Expand Down
9 changes: 6 additions & 3 deletions flow/raster_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@ namespace flow {

class RasterCacheResult {
public:
RasterCacheResult() {}
RasterCacheResult();

RasterCacheResult(sk_sp<SkImage> image, const SkRect& logical_rect)
: image_(std::move(image)), logical_rect_(logical_rect) {}
RasterCacheResult(const RasterCacheResult& other);

~RasterCacheResult();

RasterCacheResult(sk_sp<SkImage> image, const SkRect& logical_rect);

operator bool() const { return static_cast<bool>(image_); }

Expand Down
1 change: 1 addition & 0 deletions fml/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ source_set("fml") {
"task_runner.h",
"thread.cc",
"thread.h",
"thread_local.cc",
"thread_local.h",
"time/time_delta.h",
"time/time_point.cc",
Expand Down
15 changes: 15 additions & 0 deletions fml/file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,19 @@ fml::UniqueFD CreateDirectory(const fml::UniqueFD& base_directory,
return CreateDirectory(base_directory, components, permission, 0);
}

ScopedTemporaryDirectory::ScopedTemporaryDirectory() {
path_ = CreateTemporaryDirectory();
if (path_ != "") {
dir_fd_ = OpenDirectory(path_.c_str(), false, FilePermission::kRead);
}
}

ScopedTemporaryDirectory::~ScopedTemporaryDirectory() {
if (path_ != "") {
if (!UnlinkDirectory(path_.c_str())) {
FML_LOG(ERROR) << "Could not remove directory: " << path_;
}
}
}

} // namespace fml
17 changes: 3 additions & 14 deletions fml/file.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,20 +75,9 @@ bool WriteAtomically(const fml::UniqueFD& base_directory,

class ScopedTemporaryDirectory {
public:
ScopedTemporaryDirectory() {
path_ = CreateTemporaryDirectory();
if (path_ != "") {
dir_fd_ = OpenDirectory(path_.c_str(), false, FilePermission::kRead);
}
}

~ScopedTemporaryDirectory() {
if (path_ != "") {
if (!UnlinkDirectory(path_.c_str())) {
FML_LOG(ERROR) << "Could not remove directory: " << path_;
}
}
}
ScopedTemporaryDirectory();

~ScopedTemporaryDirectory();

const UniqueFD& fd() { return dir_fd_; }

Expand Down
2 changes: 1 addition & 1 deletion fml/logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const char* StripDots(const char* path) {
}

const char* StripPath(const char* path) {
auto p = strrchr(path, '/');
auto* p = strrchr(path, '/');
if (p)
return p + 1;
else
Expand Down
8 changes: 8 additions & 0 deletions fml/mapping.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,12 @@ const uint8_t* DataMapping::GetMapping() const {
return data_.data();
}

size_t NonOwnedMapping::GetSize() const {
return size_;
}

const uint8_t* NonOwnedMapping::GetMapping() const {
return data_;
}

} // namespace fml
4 changes: 2 additions & 2 deletions fml/mapping.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ class NonOwnedMapping : public Mapping {
NonOwnedMapping(const uint8_t* data, size_t size)
: data_(data), size_(size) {}

size_t GetSize() const override { return size_; }
size_t GetSize() const override;

const uint8_t* GetMapping() const override { return data_; }
const uint8_t* GetMapping() const override;

private:
const uint8_t* const data_;
Expand Down
6 changes: 5 additions & 1 deletion fml/message.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@

namespace fml {

size_t MessageSerializable::GetSerializableTag() const {
return 0;
};

Message::Message() = default;

Message::~Message() = default;
Expand Down Expand Up @@ -96,7 +100,7 @@ uint8_t* Message::PrepareDecode(size_t size) {
if ((size + size_read_) > buffer_length_) {
return nullptr;
}
auto buffer = buffer_ + size_read_;
auto* buffer = buffer_ + size_read_;
size_read_ += size;
return buffer;
}
Expand Down
6 changes: 3 additions & 3 deletions fml/message.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class MessageSerializable {

virtual bool Deserialize(Message& message) = 0;

virtual size_t GetSerializableTag() const { return 0; };
virtual size_t GetSerializableTag() const;
};

// The traits passed to the encode/decode calls that accept traits should be
Expand Down Expand Up @@ -88,7 +88,7 @@ class Message {
template <typename T,
typename = std::enable_if_t<std::is_trivially_copyable<T>::value>>
FML_WARN_UNUSED_RESULT bool Encode(const T& value) {
if (auto buffer = PrepareEncode(sizeof(T))) {
if (auto* buffer = PrepareEncode(sizeof(T))) {
::memcpy(buffer, &value, sizeof(T));
return true;
}
Expand Down Expand Up @@ -131,7 +131,7 @@ class Message {
template <typename T,
typename = std::enable_if_t<std::is_trivially_copyable<T>::value>>
FML_WARN_UNUSED_RESULT bool Decode(T& value) {
if (auto buffer = PrepareDecode(sizeof(T))) {
if (auto* buffer = PrepareDecode(sizeof(T))) {
::memcpy(&value, buffer, sizeof(T));
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion fml/message_loop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ FML_THREAD_LOCAL ThreadLocal tls_message_loop([](intptr_t value) {
});

MessageLoop& MessageLoop::GetCurrent() {
auto loop = reinterpret_cast<MessageLoop*>(tls_message_loop.Get());
auto* loop = reinterpret_cast<MessageLoop*>(tls_message_loop.Get());
FML_CHECK(loop != nullptr)
<< "MessageLoop::EnsureInitializedForCurrentThread was not called on "
"this thread prior to message loop use.";
Expand Down
9 changes: 9 additions & 0 deletions fml/message_loop_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -145,4 +145,13 @@ void MessageLoopImpl::RunExpiredTasks() {
}
}

MessageLoopImpl::DelayedTask::DelayedTask(size_t p_order,
fml::closure p_task,
fml::TimePoint p_target_time)
: order(p_order), task(std::move(p_task)), target_time(p_target_time) {}

MessageLoopImpl::DelayedTask::DelayedTask(const DelayedTask& other) = default;

MessageLoopImpl::DelayedTask::~DelayedTask() = default;

} // namespace fml
7 changes: 5 additions & 2 deletions fml/message_loop_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,11 @@ class MessageLoopImpl : public fml::RefCountedThreadSafe<MessageLoopImpl> {

DelayedTask(size_t p_order,
fml::closure p_task,
fml::TimePoint p_target_time)
: order(p_order), task(std::move(p_task)), target_time(p_target_time) {}
fml::TimePoint p_target_time);

DelayedTask(const DelayedTask& other);

~DelayedTask();
};

struct DelayedTaskCompare {
Expand Down
2 changes: 1 addition & 1 deletion fml/platform/posix/file_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace fml {

std::string CreateTemporaryDirectory() {
char directory_name[] = "/tmp/flutter_XXXXXXXX";
auto result = ::mkdtemp(directory_name);
auto* result = ::mkdtemp(directory_name);
if (result == nullptr) {
return "";
}
Expand Down
2 changes: 1 addition & 1 deletion fml/platform/posix/mapping_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ FileMapping::FileMapping(const fml::UniqueFD& handle,

const auto is_writable = IsWritable(protection);

auto mapping =
auto* mapping =
::mmap(nullptr, stat_buffer.st_size, ToPosixProtectionFlags(protection),
is_writable ? MAP_SHARED : MAP_PRIVATE, handle.get(), 0);

Expand Down
2 changes: 1 addition & 1 deletion fml/platform/posix/native_library_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ fml::RefPtr<NativeLibrary> NativeLibrary::CreateForCurrentProcess() {
}

const uint8_t* NativeLibrary::ResolveSymbol(const char* symbol) {
auto resolved_symbol = static_cast<const uint8_t*>(::dlsym(handle_, symbol));
auto* resolved_symbol = static_cast<const uint8_t*>(::dlsym(handle_, symbol));
if (resolved_symbol == nullptr) {
FML_DLOG(INFO) << "Could not resolve symbol in library: " << symbol;
}
Expand Down
8 changes: 4 additions & 4 deletions fml/string_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ size_t StringView::find(StringView s, size_t pos) const {
if (s.empty())
return pos;

auto result = std::search(begin() + pos, end(), s.begin(), s.end());
auto* result = std::search(begin() + pos, end(), s.begin(), s.end());
if (result == end())
return npos;
return result - begin();
Expand All @@ -94,7 +94,7 @@ size_t StringView::find(char c, size_t pos) const {
if (pos > size_)
return npos;

auto result = std::find(begin() + pos, end(), c);
auto* result = std::find(begin() + pos, end(), c);
if (result == end())
return npos;
return result - begin();
Expand All @@ -106,8 +106,8 @@ size_t StringView::rfind(StringView s, size_t pos) const {
if (s.empty())
return std::min(pos, size_);

auto last = begin() + std::min(size_ - s.size(), pos) + s.size();
auto result = std::find_end(begin(), last, s.begin(), s.end());
auto* last = begin() + std::min(size_ - s.size(), pos) + s.size();
auto* result = std::find_end(begin(), last, s.begin(), s.end());
if (result == last)
return npos;
return result - begin();
Expand Down
Loading

0 comments on commit 09ef73f

Please sign in to comment.