Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix minor style issues uncovered during auditing #6851

Merged
merged 7 commits into from
Oct 28, 2020
Merged
4 changes: 2 additions & 2 deletions Firestore/core/src/auth/user.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ User::User(std::string uid) : uid_{std::move(uid)}, is_authenticated_{true} {
}

const User& User::Unauthenticated() {
static const User kUnauthenticated;
return kUnauthenticated;
static const User* kUnauthenticated = new User();
return *kUnauthenticated;
}

} // namespace auth
Expand Down
10 changes: 6 additions & 4 deletions Firestore/core/src/immutable/array_sorted_map.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018 Google
* Copyright 2018 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -326,9 +326,11 @@ class ArraySortedMap : public SortedMapBase {

private:
static array_pointer EmptyArray() {
static const array_pointer kEmptyArray =
std::make_shared<const array_type>();
return kEmptyArray;
static const array_pointer* kEmptyArray = [] {
auto array = new array_type();
return new std::shared_ptr<const array_type>(array);
}();
return *kEmptyArray;
}

static array_pointer SortedArray(std::initializer_list<value_type> entries,
Expand Down
14 changes: 7 additions & 7 deletions Firestore/core/src/immutable/llrb_node.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,18 +162,18 @@ class LlrbNode : public SortedMapBase {
* Returns a shared Empty node, to cut down on allocations in the base case.
*/
static const std::shared_ptr<Rep>& EmptyRep() {
static const std::shared_ptr<Rep> empty_rep = [] {
auto empty = std::make_shared<Rep>(Rep{std::pair<K, V>{}, Color::Black,
/* size= */ 0u, LlrbNode{nullptr},
LlrbNode{nullptr}});
static const std::shared_ptr<Rep>* empty_rep = [] {
auto rep = new Rep{std::pair<K, V>{}, Color::Black,
/* size= */ 0u, LlrbNode{nullptr}, LlrbNode{nullptr}};
auto empty = new std::shared_ptr<Rep>{rep};

// Set up the empty Rep such that you can traverse infinitely down left
// and right links.
empty->left_.rep_ = empty;
empty->right_.rep_ = empty;
(*empty)->left_.rep_ = *empty;
(*empty)->right_.rep_ = *empty;
return empty;
}();
return empty_rep;
return *empty_rep;
}

/**
Expand Down
5 changes: 2 additions & 3 deletions Firestore/core/src/local/leveldb_migrations.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018 Google
* Copyright 2018 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -33,6 +33,7 @@
namespace firebase {
namespace firestore {
namespace local {
namespace {

using leveldb::Iterator;
using leveldb::Slice;
Expand All @@ -44,8 +45,6 @@ using nanopb::Message;
using nanopb::StringReader;
using nanopb::Writer;

namespace {

/**
* Schema version for the iOS client.
*
Expand Down
2 changes: 2 additions & 0 deletions Firestore/core/src/local/leveldb_transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ LevelDbTransaction::LevelDbTransaction(DB* db,
}

const ReadOptions& LevelDbTransaction::DefaultReadOptions() {
// ReadOptions is trivial so it does not need to be heap-allocated.
static ReadOptions options = [] {
ReadOptions read_options;
read_options.verify_checksums = true;
Expand All @@ -155,6 +156,7 @@ const ReadOptions& LevelDbTransaction::DefaultReadOptions() {
}

const WriteOptions& LevelDbTransaction::DefaultWriteOptions() {
// WriteOptions is trivial so it does not need to be heap-allocated.
static WriteOptions options;
return options;
}
Expand Down
3 changes: 1 addition & 2 deletions Firestore/core/src/local/leveldb_util.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018 Google
* Copyright 2018 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -23,7 +23,6 @@
namespace firebase {
namespace firestore {
namespace local {

namespace {

Error ConvertStatusCode(const leveldb::Status& status) {
Expand Down
3 changes: 1 addition & 2 deletions Firestore/core/src/local/local_serializer.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018 Google
* Copyright 2018 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -43,7 +43,6 @@
namespace firebase {
namespace firestore {
namespace local {

namespace {

using core::Target;
Expand Down
3 changes: 1 addition & 2 deletions Firestore/core/src/local/local_store.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2019 Google
* Copyright 2019 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -37,7 +37,6 @@
namespace firebase {
namespace firestore {
namespace local {

namespace {

using auth::User;
Expand Down
5 changes: 2 additions & 3 deletions Firestore/core/src/local/target_data.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018 Google
* Copyright 2018 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -23,6 +23,7 @@
namespace firebase {
namespace firestore {
namespace local {
namespace {

using core::Target;
using model::ListenSequenceNumber;
Expand All @@ -32,8 +33,6 @@ using nanopb::ByteString;

// MARK: - QueryPurpose

namespace {

const char* ToString(QueryPurpose purpose) {
switch (purpose) {
case QueryPurpose::Listen:
Expand Down
7 changes: 3 additions & 4 deletions Firestore/core/src/model/document_key.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018 Google
* Copyright 2018 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -27,7 +27,6 @@
namespace firebase {
namespace firestore {
namespace model {

namespace {

void AssertValidPath(const ResourcePath& path) {
Expand Down Expand Up @@ -59,8 +58,8 @@ DocumentKey DocumentKey::FromSegments(std::initializer_list<std::string> list) {
}

const DocumentKey& DocumentKey::Empty() {
static const DocumentKey empty;
return empty;
static const DocumentKey* empty = new DocumentKey();
return *empty;
}

bool DocumentKey::IsDocumentKey(const ResourcePath& path) {
Expand Down
6 changes: 3 additions & 3 deletions Firestore/core/src/model/field_path.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018 Google
* Copyright 2018 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -28,11 +28,10 @@
namespace firebase {
namespace firestore {
namespace model {
namespace {

using util::ThrowInvalidArgument;

namespace {

/**
* True if the string could be used as a segment in a field path without
* escaping. Valid identifies follow the regex [a-zA-Z_][a-zA-Z0-9_]*
Expand Down Expand Up @@ -79,6 +78,7 @@ struct JoinEscaped {
out->append(escaped_segment(segment));
}
};

} // namespace

constexpr const char* FieldPath::kDocumentKeyPath;
Expand Down
3 changes: 1 addition & 2 deletions Firestore/core/src/model/field_value.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018 Google
* Copyright 2018 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -41,7 +41,6 @@
namespace firebase {
namespace firestore {
namespace model {

namespace {

using BaseValue = FieldValue::BaseValue;
Expand Down
3 changes: 1 addition & 2 deletions Firestore/core/src/nanopb/pretty_printing.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2019 Google
* Copyright 2019 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -23,7 +23,6 @@
namespace firebase {
namespace firestore {
namespace nanopb {

namespace internal {

std::string Indent(int level, int indent_width) {
Expand Down
15 changes: 7 additions & 8 deletions Firestore/core/src/nanopb/writer.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018 Google
* Copyright 2018 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -25,13 +25,6 @@
namespace firebase {
namespace firestore {
namespace nanopb {

void Writer::Write(const pb_field_t fields[], const void* src_struct) {
if (!pb_encode(&stream_, fields, src_struct)) {
HARD_FAIL(PB_GET_ERROR(&stream_));
}
}

namespace {

constexpr size_t kMinBufferSize = 4;
Expand All @@ -46,6 +39,12 @@ bool AppendToBytesArray(pb_ostream_t* stream,

} // namespace

void Writer::Write(const pb_field_t fields[], const void* src_struct) {
if (!pb_encode(&stream_, fields, src_struct)) {
HARD_FAIL(PB_GET_ERROR(&stream_));
}
}

ByteStringWriter::ByteStringWriter() {
stream_.callback = AppendToBytesArray;
stream_.state = this;
Expand Down
11 changes: 6 additions & 5 deletions Firestore/core/src/remote/datastore.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ void LogGrpcCallFinished(absl::string_view rpc_name,
status.error_message());
if (LogIsDebugEnabled()) {
auto headers =
Datastore::GetWhitelistedHeadersAsString(call->GetResponseHeaders());
LOG_DEBUG("RPC %s returned headers (whitelisted): %s", rpc_name, headers);
Datastore::GetAllowlistedHeadersAsString(call->GetResponseHeaders());
LOG_DEBUG("RPC %s returned headers (allowlisted): %s", rpc_name, headers);
}
}

Expand Down Expand Up @@ -332,15 +332,16 @@ bool Datastore::IsPermanentWriteError(const Status& error) {
return IsPermanentError(error) && !IsAbortedError(error);
}

std::string Datastore::GetWhitelistedHeadersAsString(
std::string Datastore::GetAllowlistedHeadersAsString(
const GrpcCall::Metadata& headers) {
static std::unordered_set<std::string> whitelist = {
static auto* allowlist = new std::unordered_set<std::string>{
"date", "x-google-backends", "x-google-netmon-label", "x-google-service",
"x-google-gfe-request-trace"};

std::string result;
auto end = allowlist->end();
for (const auto& kv : headers) {
if (whitelist.find(MakeString(kv.first)) != whitelist.end()) {
if (allowlist->find(MakeString(kv.first)) != end) {
absl::StrAppend(&result, MakeStringView(kv.first), ": ",
MakeStringView(kv.second), "\n");
}
Expand Down
4 changes: 2 additions & 2 deletions Firestore/core/src/remote/datastore.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ class Datastore : public std::enable_shared_from_this<Datastore> {
*/
static bool IsPermanentWriteError(const util::Status& status);

static std::string GetWhitelistedHeadersAsString(
static std::string GetAllowlistedHeadersAsString(
const GrpcCall::Metadata& headers);

Datastore(const Datastore& other) = delete;
Expand Down Expand Up @@ -169,7 +169,7 @@ class Datastore : public std::enable_shared_from_this<Datastore> {

void RemoveGrpcCall(GrpcCall* to_remove);

static GrpcCall::Metadata ExtractWhitelistedHeaders(
static GrpcCall::Metadata ExtractAllowlistedHeaders(
const GrpcCall::Metadata& headers);

// In case Auth tries to invoke a callback after `Datastore` has been shut
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
namespace firebase {
namespace firestore {
namespace remote {

namespace {

using util::MakeString;
Expand Down
13 changes: 6 additions & 7 deletions Firestore/core/src/remote/grpc_connection.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018 Google
* Copyright 2018 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -45,6 +45,7 @@ SUPPRESS_END()
namespace firebase {
namespace firestore {
namespace remote {
namespace {

using auth::Token;
using core::DatabaseInfo;
Expand All @@ -55,8 +56,6 @@ using util::Status;
using util::StatusOr;
using util::StringFormat;

namespace {

const char* const kAuthorizationHeader = "authorization";
const char* const kXGoogApiClientHeader = "x-goog-api-client";
const char* const kGoogleCloudResourcePrefix = "google-cloud-resource-prefix";
Expand Down Expand Up @@ -125,8 +124,8 @@ class HostConfigMap {
};

HostConfigMap& Config() {
static HostConfigMap config_by_host;
return config_by_host;
static auto* config_by_host = new HostConfigMap();
return *config_by_host;
}

std::string GetCppLanguageToken() {
Expand Down Expand Up @@ -170,8 +169,8 @@ class ClientLanguageToken {
};

ClientLanguageToken& LanguageToken() {
static ClientLanguageToken token;
return token;
static auto* token = new ClientLanguageToken();
return *token;
}

void AddCloudApiHeader(grpc::ClientContext& context) {
Expand Down
Loading