[PATCH] D38082: [yaml2obj] - Don't crash on one more invalid document.

[Rafael Avila de Espindola via llvm-commits]

LGTM

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar updated this revision to Diff 116008.
> grimar added a comment.
>
> - Fixed last minute renaming mistake.
>
>
> https://reviews.llvm.org/D38082
>
> Files:
>   lib/Support/YAMLTraits.cpp
>   test/Object/yaml2obj-invalid.yaml
>
>
> Index: test/Object/yaml2obj-invalid.yaml
> ===================================================================
> --- test/Object/yaml2obj-invalid.yaml
> +++ test/Object/yaml2obj-invalid.yaml
> @@ -0,0 +1,4 @@
> +AAA: | BBB
> +
> +# RUN: not yaml2obj %s 2>&1 | FileCheck %s
> +# CHECK: Map value must not be empty
> Index: lib/Support/YAMLTraits.cpp
> ===================================================================
> --- lib/Support/YAMLTraits.cpp
> +++ lib/Support/YAMLTraits.cpp
> @@ -374,18 +374,22 @@
>      auto mapHNode = llvm::make_unique<MapHNode>(N);
>      for (KeyValueNode &KVN : *Map) {
>        Node *KeyNode = KVN.getKey();
> -      ScalarNode *KeyScalar = dyn_cast<ScalarNode>(KeyNode);
> -      if (!KeyScalar) {
> -        setError(KeyNode, "Map key must be a scalar");
> +      ScalarNode *Key = dyn_cast<ScalarNode>(KeyNode);
> +      Node *Value = KVN.getValue();
> +      if (!Key || !Value) {
> +        if (!Key)
> +          setError(KeyNode, "Map key must be a scalar");
> +        if (!Value)
> +          setError(KeyNode, "Map value must not be empty");
>          break;
>        }
>        StringStorage.clear();
> -      StringRef KeyStr = KeyScalar->getValue(StringStorage);
> +      StringRef KeyStr = Key->getValue(StringStorage);
>        if (!StringStorage.empty()) {
>          // Copy string to permanent storage
>          KeyStr = StringStorage.str().copy(StringAllocator);
>        }
> -      auto ValueHNode = this->createHNodes(KVN.getValue());
> +      auto ValueHNode = this->createHNodes(Value);
>        if (EC)
>          break;
>        mapHNode->Mapping[KeyStr] = std::move(ValueHNode);
>
>
> Index: test/Object/yaml2obj-invalid.yaml
> ===================================================================
> --- test/Object/yaml2obj-invalid.yaml
> +++ test/Object/yaml2obj-invalid.yaml
> @@ -0,0 +1,4 @@
> +AAA: | BBB
> +
> +# RUN: not yaml2obj %s 2>&1 | FileCheck %s
> +# CHECK: Map value must not be empty
> Index: lib/Support/YAMLTraits.cpp
> ===================================================================
> --- lib/Support/YAMLTraits.cpp
> +++ lib/Support/YAMLTraits.cpp
> @@ -374,18 +374,22 @@
>      auto mapHNode = llvm::make_unique<MapHNode>(N);
>      for (KeyValueNode &KVN : *Map) {
>        Node *KeyNode = KVN.getKey();
> -      ScalarNode *KeyScalar = dyn_cast<ScalarNode>(KeyNode);
> -      if (!KeyScalar) {
> -        setError(KeyNode, "Map key must be a scalar");
> +      ScalarNode *Key = dyn_cast<ScalarNode>(KeyNode);
> +      Node *Value = KVN.getValue();
> +      if (!Key || !Value) {
> +        if (!Key)
> +          setError(KeyNode, "Map key must be a scalar");
> +        if (!Value)
> +          setError(KeyNode, "Map value must not be empty");
>          break;
>        }
>        StringStorage.clear();
> -      StringRef KeyStr = KeyScalar->getValue(StringStorage);
> +      StringRef KeyStr = Key->getValue(StringStorage);
>        if (!StringStorage.empty()) {
>          // Copy string to permanent storage
>          KeyStr = StringStorage.str().copy(StringAllocator);
>        }
> -      auto ValueHNode = this->createHNodes(KVN.getValue());
> +      auto ValueHNode = this->createHNodes(Value);
>        if (EC)
>          break;
>        mapHNode->Mapping[KeyStr] = std::move(ValueHNode);