Skip to content

Commit

Permalink
Refactor Option obj address from char* to void* (facebook#8295)
Browse files Browse the repository at this point in the history
Summary:
And replace `reinterpret_cast` with `static_cast` or no cast.

Pull Request resolved: facebook#8295

Test Plan: `make check`

Reviewed By: mrambacher

Differential Revision: D28420303

Pulled By: jay-zhuang

fbshipit-source-id: 645be123a0df624dc2bea37cd54a35403fc494fa
  • Loading branch information
jay-zhuang authored and facebook-github-bot committed May 13, 2021
1 parent d76c46e commit d15fbae
Show file tree
Hide file tree
Showing 8 changed files with 177 additions and 194 deletions.
6 changes: 3 additions & 3 deletions cache/cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ Status Cache::CreateFromString(const ConfigOptions& config_options,
} else {
#ifndef ROCKSDB_LITE
LRUCacheOptions cache_opts;
status = OptionTypeInfo::ParseStruct(
config_options, "", &lru_cache_options_type_info, "", value,
reinterpret_cast<char*>(&cache_opts));
status = OptionTypeInfo::ParseStruct(config_options, "",
&lru_cache_options_type_info, "",
value, &cache_opts);
if (status.ok()) {
cache = NewLRUCache(cache_opts);
}
Expand Down
35 changes: 16 additions & 19 deletions db/compaction/compaction_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2102,27 +2102,24 @@ static std::unordered_map<std::string, OptionTypeInfo> cfd_type_info = {
{offset_of(&ColumnFamilyDescriptor::options), OptionType::kConfigurable,
OptionVerificationType::kNormal, OptionTypeFlags::kNone,
[](const ConfigOptions& opts, const std::string& /*name*/,
const std::string& value, char* addr) {
auto cf_options = reinterpret_cast<ColumnFamilyOptions*>(addr);
const std::string& value, void* addr) {
auto cf_options = static_cast<ColumnFamilyOptions*>(addr);
return GetColumnFamilyOptionsFromString(opts, ColumnFamilyOptions(),
value, cf_options);
},
[](const ConfigOptions& opts, const std::string& /*name*/,
const char* addr, std::string* value) {
const auto cf_options =
reinterpret_cast<const ColumnFamilyOptions*>(addr);
const void* addr, std::string* value) {
const auto cf_options = static_cast<const ColumnFamilyOptions*>(addr);
std::string result;
auto status =
GetStringFromColumnFamilyOptions(opts, *cf_options, &result);
*value = "{" + result + "}";
return status;
},
[](const ConfigOptions& opts, const std::string& name, const char* addr1,
const char* addr2, std::string* mismatch) {
const auto this_one =
reinterpret_cast<const ColumnFamilyOptions*>(addr1);
const auto that_one =
reinterpret_cast<const ColumnFamilyOptions*>(addr2);
[](const ConfigOptions& opts, const std::string& name, const void* addr1,
const void* addr2, std::string* mismatch) {
const auto this_one = static_cast<const ColumnFamilyOptions*>(addr1);
const auto that_one = static_cast<const ColumnFamilyOptions*>(addr2);
auto this_conf = CFOptionsAsConfigurable(*this_one);
auto that_conf = CFOptionsAsConfigurable(*that_one);
std::string mismatch_opt;
Expand All @@ -2145,22 +2142,22 @@ static std::unordered_map<std::string, OptionTypeInfo> cs_input_type_info = {
{offset_of(&CompactionServiceInput::db_options), OptionType::kConfigurable,
OptionVerificationType::kNormal, OptionTypeFlags::kNone,
[](const ConfigOptions& opts, const std::string& /*name*/,
const std::string& value, char* addr) {
auto options = reinterpret_cast<DBOptions*>(addr);
const std::string& value, void* addr) {
auto options = static_cast<DBOptions*>(addr);
return GetDBOptionsFromString(opts, DBOptions(), value, options);
},
[](const ConfigOptions& opts, const std::string& /*name*/,
const char* addr, std::string* value) {
const auto options = reinterpret_cast<const DBOptions*>(addr);
const void* addr, std::string* value) {
const auto options = static_cast<const DBOptions*>(addr);
std::string result;
auto status = GetStringFromDBOptions(opts, *options, &result);
*value = "{" + result + "}";
return status;
},
[](const ConfigOptions& opts, const std::string& name, const char* addr1,
const char* addr2, std::string* mismatch) {
const auto this_one = reinterpret_cast<const DBOptions*>(addr1);
const auto that_one = reinterpret_cast<const DBOptions*>(addr2);
[](const ConfigOptions& opts, const std::string& name, const void* addr1,
const void* addr2, std::string* mismatch) {
const auto this_one = static_cast<const DBOptions*>(addr1);
const auto that_one = static_cast<const DBOptions*>(addr2);
auto this_conf = DBOptionsAsConfigurable(*this_one);
auto that_conf = DBOptionsAsConfigurable(*that_one);
std::string mismatch_opt;
Expand Down
93 changes: 46 additions & 47 deletions include/rocksdb/utilities/options_type.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ bool VectorsAreEqual(const ConfigOptions& config_options,
// @param addr Pointer to the object
using ParseFunc = std::function<Status(
const ConfigOptions& /*opts*/, const std::string& /*name*/,
const std::string& /*value*/, char* /*addr*/)>;
const std::string& /*value*/, void* /*addr*/)>;

// Function for converting an option "addr" into its string representation.
// On success, Status::OK is returned and value is the serialized form.
Expand All @@ -188,7 +188,7 @@ using ParseFunc = std::function<Status(
// @param value The result of the serialization.
using SerializeFunc = std::function<Status(
const ConfigOptions& /*opts*/, const std::string& /*name*/,
const char* /*addr*/, std::string* /*value*/)>;
const void* /*addr*/, std::string* /*value*/)>;

// Function for comparing two option values
// If they are not equal, updates "mismatch" with the name of the bad option
Expand All @@ -200,7 +200,7 @@ using SerializeFunc = std::function<Status(
// first differs
using EqualsFunc = std::function<bool(
const ConfigOptions& /*opts*/, const std::string& /*name*/,
const char* /*addr1*/, const char* /*addr2*/, std::string* mismatch)>;
const void* /*addr1*/, const void* /*addr2*/, std::string* mismatch)>;

// A struct for storing constant option information such as option name,
// option type, and offset.
Expand Down Expand Up @@ -273,10 +273,10 @@ class OptionTypeInfo {
// @return OK if the value is found in the map
// @return InvalidArgument if the value is not found in the map
[map](const ConfigOptions&, const std::string& name,
const std::string& value, char* addr) {
const std::string& value, void* addr) {
if (map == nullptr) {
return Status::NotSupported("No enum mapping ", name);
} else if (ParseEnum<T>(*map, value, reinterpret_cast<T*>(addr))) {
} else if (ParseEnum<T>(*map, value, static_cast<T*>(addr))) {
return Status::OK();
} else {
return Status::InvalidArgument("No mapping for enum ", name);
Expand All @@ -287,11 +287,11 @@ class OptionTypeInfo {
// value is updated to the corresponding string value in the map.
// @return OK if the enum is found in the map
// @return InvalidArgument if the enum is not found in the map
[map](const ConfigOptions&, const std::string& name, const char* addr,
[map](const ConfigOptions&, const std::string& name, const void* addr,
std::string* value) {
if (map == nullptr) {
return Status::NotSupported("No enum mapping ", name);
} else if (SerializeEnum<T>(*map, (*reinterpret_cast<const T*>(addr)),
} else if (SerializeEnum<T>(*map, (*static_cast<const T*>(addr)),
value)) {
return Status::OK();
} else {
Expand All @@ -300,10 +300,10 @@ class OptionTypeInfo {
},
// Casts addr1 and addr2 to the enum type and returns true if
// they are equal, false otherwise.
[](const ConfigOptions&, const std::string&, const char* addr1,
const char* addr2, std::string*) {
return (*reinterpret_cast<const T*>(addr1) ==
*reinterpret_cast<const T*>(addr2));
[](const ConfigOptions&, const std::string&, const void* addr1,
const void* addr2, std::string*) {
return (*static_cast<const T*>(addr1) ==
*static_cast<const T*>(addr2));
});
} // End OptionTypeInfo::Enum

Expand Down Expand Up @@ -338,20 +338,20 @@ class OptionTypeInfo {
// Parses the struct and updates the fields at addr
[struct_name, struct_map](const ConfigOptions& opts,
const std::string& name,
const std::string& value, char* addr) {
const std::string& value, void* addr) {
return ParseStruct(opts, struct_name, struct_map, name, value, addr);
},
// Serializes the struct options into value
[struct_name, struct_map](const ConfigOptions& opts,
const std::string& name, const char* addr,
const std::string& name, const void* addr,
std::string* value) {
return SerializeStruct(opts, struct_name, struct_map, name, addr,
value);
},
// Compares the struct fields of addr1 and addr2 for equality
[struct_name, struct_map](const ConfigOptions& opts,
const std::string& name, const char* addr1,
const char* addr2, std::string* mismatch) {
const std::string& name, const void* addr1,
const void* addr2, std::string* mismatch) {
return StructsAreEqual(opts, struct_name, struct_map, name, addr1,
addr2, mismatch);
});
Expand All @@ -364,14 +364,14 @@ class OptionTypeInfo {
return OptionTypeInfo(
offset, OptionType::kStruct, verification, flags, parse_func,
[struct_name, struct_map](const ConfigOptions& opts,
const std::string& name, const char* addr,
const std::string& name, const void* addr,
std::string* value) {
return SerializeStruct(opts, struct_name, struct_map, name, addr,
value);
},
[struct_name, struct_map](const ConfigOptions& opts,
const std::string& name, const char* addr1,
const char* addr2, std::string* mismatch) {
const std::string& name, const void* addr1,
const void* addr2, std::string* mismatch) {
return StructsAreEqual(opts, struct_name, struct_map, name, addr1,
addr2, mismatch);
});
Expand All @@ -387,23 +387,23 @@ class OptionTypeInfo {
_offset, OptionType::kVector, _verification, _flags,
[elem_info, separator](const ConfigOptions& opts,
const std::string& name,
const std::string& value, char* addr) {
auto result = reinterpret_cast<std::vector<T>*>(addr);
const std::string& value, void* addr) {
auto result = static_cast<std::vector<T>*>(addr);
return ParseVector<T>(opts, elem_info, separator, name, value,
result);
},
[elem_info, separator](const ConfigOptions& opts,
const std::string& name, const char* addr,
const std::string& name, const void* addr,
std::string* value) {
const auto& vec = *(reinterpret_cast<const std::vector<T>*>(addr));
const auto& vec = *(static_cast<const std::vector<T>*>(addr));
return SerializeVector<T>(opts, elem_info, separator, name, vec,
value);
},
[elem_info](const ConfigOptions& opts, const std::string& name,
const char* addr1, const char* addr2,
const void* addr1, const void* addr2,
std::string* mismatch) {
const auto& vec1 = *(reinterpret_cast<const std::vector<T>*>(addr1));
const auto& vec2 = *(reinterpret_cast<const std::vector<T>*>(addr2));
const auto& vec1 = *(static_cast<const std::vector<T>*>(addr1));
const auto& vec2 = *(static_cast<const std::vector<T>*>(addr2));
return VectorsAreEqual<T>(opts, elem_info, name, vec1, vec2,
mismatch);
});
Expand Down Expand Up @@ -435,8 +435,8 @@ class OptionTypeInfo {
offset, OptionType::kCustomizable, ovt,
flags | OptionTypeFlags::kShared,
[](const ConfigOptions& opts, const std::string&,
const std::string& value, char* addr) {
auto* shared = reinterpret_cast<std::shared_ptr<T>*>(addr);
const std::string& value, void* addr) {
auto* shared = static_cast<std::shared_ptr<T>*>(addr);
return T::CreateFromString(opts, value, shared);
},
serialize_func, equals_func);
Expand Down Expand Up @@ -468,8 +468,8 @@ class OptionTypeInfo {
offset, OptionType::kCustomizable, ovt,
flags | OptionTypeFlags::kUnique,
[](const ConfigOptions& opts, const std::string&,
const std::string& value, char* addr) {
auto* unique = reinterpret_cast<std::unique_ptr<T>*>(addr);
const std::string& value, void* addr) {
auto* unique = static_cast<std::unique_ptr<T>*>(addr);
return T::CreateFromString(opts, value, unique);
},
serialize_func, equals_func);
Expand Down Expand Up @@ -499,8 +499,8 @@ class OptionTypeInfo {
offset, OptionType::kCustomizable, ovt,
flags | OptionTypeFlags::kRawPointer,
[](const ConfigOptions& opts, const std::string&,
const std::string& value, char* addr) {
auto** pointer = reinterpret_cast<T**>(addr);
const std::string& value, void* addr) {
auto** pointer = static_cast<T**>(addr);
return T::CreateFromString(opts, value, pointer);
},
serialize_func, equals_func);
Expand Down Expand Up @@ -596,20 +596,20 @@ class OptionTypeInfo {
if (base_addr == nullptr) {
return nullptr;
}
const auto opt_addr = reinterpret_cast<const char*>(base_addr) + offset_;
const void* opt_addr = static_cast<const char*>(base_addr) + offset_;
if (IsUniquePtr()) {
const std::unique_ptr<T>* ptr =
reinterpret_cast<const std::unique_ptr<T>*>(opt_addr);
static_cast<const std::unique_ptr<T>*>(opt_addr);
return ptr->get();
} else if (IsSharedPtr()) {
const std::shared_ptr<T>* ptr =
reinterpret_cast<const std::shared_ptr<T>*>(opt_addr);
static_cast<const std::shared_ptr<T>*>(opt_addr);
return ptr->get();
} else if (IsRawPtr()) {
const T* const* ptr = reinterpret_cast<const T* const*>(opt_addr);
const T* const* ptr = static_cast<const T* const*>(opt_addr);
return *ptr;
} else {
return reinterpret_cast<const T*>(opt_addr);
return static_cast<const T*>(opt_addr);
}
}

Expand All @@ -620,18 +620,18 @@ class OptionTypeInfo {
if (base_addr == nullptr) {
return nullptr;
}
auto opt_addr = reinterpret_cast<char*>(base_addr) + offset_;
void* opt_addr = static_cast<char*>(base_addr) + offset_;
if (IsUniquePtr()) {
std::unique_ptr<T>* ptr = reinterpret_cast<std::unique_ptr<T>*>(opt_addr);
std::unique_ptr<T>* ptr = static_cast<std::unique_ptr<T>*>(opt_addr);
return ptr->get();
} else if (IsSharedPtr()) {
std::shared_ptr<T>* ptr = reinterpret_cast<std::shared_ptr<T>*>(opt_addr);
std::shared_ptr<T>* ptr = static_cast<std::shared_ptr<T>*>(opt_addr);
return ptr->get();
} else if (IsRawPtr()) {
T** ptr = reinterpret_cast<T**>(opt_addr);
T** ptr = static_cast<T**>(opt_addr);
return *ptr;
} else {
return reinterpret_cast<T*>(opt_addr);
return static_cast<T*>(opt_addr);
}
}

Expand Down Expand Up @@ -704,7 +704,7 @@ class OptionTypeInfo {
static Status ParseStruct(
const ConfigOptions& config_options, const std::string& struct_name,
const std::unordered_map<std::string, OptionTypeInfo>* map,
const std::string& opt_name, const std::string& value, char* opt_addr);
const std::string& opt_name, const std::string& value, void* opt_addr);

// Serializes the values from opt_addr using the rules in type_map.
// Returns the serialized form in result.
Expand All @@ -721,7 +721,7 @@ class OptionTypeInfo {
static Status SerializeStruct(
const ConfigOptions& config_options, const std::string& struct_name,
const std::unordered_map<std::string, OptionTypeInfo>* map,
const std::string& opt_name, const char* opt_addr, std::string* value);
const std::string& opt_name, const void* opt_addr, std::string* value);

// Compares the values in this_addr and that_addr using the rules in type_map.
// If the values are equal, returns true
Expand All @@ -740,8 +740,8 @@ class OptionTypeInfo {
static bool StructsAreEqual(
const ConfigOptions& config_options, const std::string& struct_name,
const std::unordered_map<std::string, OptionTypeInfo>* map,
const std::string& opt_name, const char* this_offset,
const char* that_offset, std::string* mismatch);
const std::string& opt_name, const void* this_offset,
const void* that_offset, std::string* mismatch);

// Finds the entry for the opt_name in the opt_map, returning
// nullptr if not found.
Expand Down Expand Up @@ -831,8 +831,7 @@ Status ParseVector(const ConfigOptions& config_options,
status = OptionTypeInfo::NextToken(value, separator, start, &end, &token);
if (status.ok()) {
T elem;
status =
elem_info.Parse(copy, name, token, reinterpret_cast<char*>(&elem));
status = elem_info.Parse(copy, name, token, &elem);
if (status.ok()) {
result->emplace_back(elem);
} else if (config_options.ignore_unsupported_options &&
Expand Down
Loading

0 comments on commit d15fbae

Please sign in to comment.