[PATCH] D61608: YAML parser robustness improvements
Don Hinton via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 4 16:15:24 PST 2019
hintonda requested changes to this revision.
hintonda added inline comments.
This revision now requires changes to proceed.
================
Comment at: llvm/lib/Support/YAMLTraits.cpp:373
SmallString<128> StringStorage;
- if (ScalarNode *SN = dyn_cast<ScalarNode>(N)) {
+ if (ScalarNode *SN = dyn_cast_or_null<ScalarNode>(N)) {
StringRef KeyStr = SN->getValue(StringStorage);
----------------
Since `createHNodes` is an internal function and only ever called 3 times within `Input::` (and nowhere else), and `N` is already checked before each of those calls, there's no need add `_of_null` to these `dyn_cast` calls and check for null again each time.
To enforce this, I'd suggest you make `createHNodes` and `setError` private which will make the class more robust, e.g.:
```
index f365d60b3ff..fe2fe8c56da 100644
--- a/llvm/include/llvm/Support/YAMLTraits.h
+++ b/llvm/include/llvm/Support/YAMLTraits.h
@@ -1509,6 +1509,7 @@ private:
std::vector<std::unique_ptr<HNode>> Entries;
};
+private:
std::unique_ptr<Input::HNode> createHNodes(Node *node);
void setError(HNode *hnode, const Twine &message);
void setError(Node *node, const Twine &message);
```
However, `KeyNode` on line 396 could be null, so either checking or adding `_or_null` to the call is definitely needed there.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61608/new/
https://reviews.llvm.org/D61608
More information about the llvm-commits
mailing list