Commit 4ea5490a authored by mgiuca's avatar mgiuca Committed by Commit bot

JSONStringValueSerializer: Constructor argument must not be null.

Previously, this class would fail gracefully if given a nullptr. Now it
has undefined behaviour. Dealing with null isn't necessary as no caller
passes nullptr here, and in general, pass-by-result style functions do
not expect null pointers. Dealing with nullptr makes my follow-up work
messy, so I am just removing it.

Also, moved constructor implementation to .cc file, and properly
documented the lifetime constraints on |json_string|.

BUG=455068

Review URL: https://codereview.chromium.org/889303004

Cr-Commit-Position: refs/heads/master@{#314703}
parent c919cea5
...@@ -10,6 +10,22 @@ ...@@ -10,6 +10,22 @@
using base::Value; using base::Value;
JSONStringValueSerializer::JSONStringValueSerializer(std::string* json_string)
: json_string_(json_string),
initialized_with_const_string_(false),
pretty_print_(false),
allow_trailing_comma_(false) {
DCHECK(json_string);
}
JSONStringValueSerializer::JSONStringValueSerializer(
const std::string& json_string)
: json_string_(&const_cast<std::string&>(json_string)),
initialized_with_const_string_(true),
pretty_print_(false),
allow_trailing_comma_(false) {
}
JSONStringValueSerializer::~JSONStringValueSerializer() {} JSONStringValueSerializer::~JSONStringValueSerializer() {}
bool JSONStringValueSerializer::Serialize(const Value& root) { bool JSONStringValueSerializer::Serialize(const Value& root) {
...@@ -23,7 +39,7 @@ bool JSONStringValueSerializer::SerializeAndOmitBinaryValues( ...@@ -23,7 +39,7 @@ bool JSONStringValueSerializer::SerializeAndOmitBinaryValues(
bool JSONStringValueSerializer::SerializeInternal(const Value& root, bool JSONStringValueSerializer::SerializeInternal(const Value& root,
bool omit_binary_values) { bool omit_binary_values) {
if (!json_string_ || initialized_with_const_string_) if (initialized_with_const_string_)
return false; return false;
int options = 0; int options = 0;
...@@ -37,9 +53,6 @@ bool JSONStringValueSerializer::SerializeInternal(const Value& root, ...@@ -37,9 +53,6 @@ bool JSONStringValueSerializer::SerializeInternal(const Value& root,
Value* JSONStringValueSerializer::Deserialize(int* error_code, Value* JSONStringValueSerializer::Deserialize(int* error_code,
std::string* error_str) { std::string* error_str) {
if (!json_string_)
return NULL;
return base::JSONReader::ReadAndReturnError(*json_string_, return base::JSONReader::ReadAndReturnError(*json_string_,
allow_trailing_comma_ ? base::JSON_ALLOW_TRAILING_COMMAS : allow_trailing_comma_ ? base::JSON_ALLOW_TRAILING_COMMAS :
base::JSON_PARSE_RFC, base::JSON_PARSE_RFC,
......
...@@ -14,24 +14,15 @@ ...@@ -14,24 +14,15 @@
class BASE_EXPORT JSONStringValueSerializer : public base::ValueSerializer { class BASE_EXPORT JSONStringValueSerializer : public base::ValueSerializer {
public: public:
// json_string is the string that will be source of the deserialization // |json_string| is the string that will be source of the deserialization
// or the destination of the serialization. The caller of the constructor // or the destination of the serialization. The caller of the constructor
// retains ownership of the string. // retains ownership of the string. |json_string| must not be null.
explicit JSONStringValueSerializer(std::string* json_string) explicit JSONStringValueSerializer(std::string* json_string);
: json_string_(json_string),
initialized_with_const_string_(false),
pretty_print_(false),
allow_trailing_comma_(false) {
}
// This version allows initialization with a const string reference for // This version allows initialization with a const string reference for
// deserialization only. // deserialization only. Retains a reference to |json_string|, so the string
explicit JSONStringValueSerializer(const std::string& json_string) // argument must outlive the JSONStringValueSerializer.
: json_string_(&const_cast<std::string&>(json_string)), explicit JSONStringValueSerializer(const std::string& json_string);
initialized_with_const_string_(true),
pretty_print_(false),
allow_trailing_comma_(false) {
}
~JSONStringValueSerializer() override; ~JSONStringValueSerializer() override;
...@@ -46,7 +37,7 @@ class BASE_EXPORT JSONStringValueSerializer : public base::ValueSerializer { ...@@ -46,7 +37,7 @@ class BASE_EXPORT JSONStringValueSerializer : public base::ValueSerializer {
// Attempt to deserialize the data structure encoded in the string passed // Attempt to deserialize the data structure encoded in the string passed
// in to the constructor into a structure of Value objects. If the return // in to the constructor into a structure of Value objects. If the return
// value is NULL, and if |error_code| is non-null, |error_code| will // value is null, and if |error_code| is non-null, |error_code| will
// contain an integer error code (a JsonParseError in this case). // contain an integer error code (a JsonParseError in this case).
// If |error_message| is non-null, it will be filled in with a formatted // If |error_message| is non-null, it will be filled in with a formatted
// error message including the location of the error if appropriate. // error message including the location of the error if appropriate.
...@@ -64,7 +55,7 @@ class BASE_EXPORT JSONStringValueSerializer : public base::ValueSerializer { ...@@ -64,7 +55,7 @@ class BASE_EXPORT JSONStringValueSerializer : public base::ValueSerializer {
private: private:
bool SerializeInternal(const base::Value& root, bool omit_binary_values); bool SerializeInternal(const base::Value& root, bool omit_binary_values);
std::string* json_string_; std::string* json_string_; // Not null.
bool initialized_with_const_string_; bool initialized_with_const_string_;
bool pretty_print_; // If true, serialization will span multiple lines. bool pretty_print_; // If true, serialization will span multiple lines.
// If true, deserialization will allow trailing commas. // If true, deserialization will allow trailing commas.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment