Document non-null entries for CPDF_Dictionary
Then remove one small redundant check.
-- Avoid assignment in conditional while at it.
Change-Id: I885892b8a8ed40dffeb555da035f07252feaee92
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/87412
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_dictionary.cpp b/core/fpdfapi/parser/cpdf_dictionary.cpp
index a798b6e..fc671e7 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.cpp
+++ b/core/fpdfapi/parser/cpdf_dictionary.cpp
@@ -33,7 +33,7 @@
// and break cyclic references.
m_ObjNum = kInvalidObjNum;
for (auto& it : m_Map) {
- if (it.second && it.second->GetObjNum() == kInvalidObjNum)
+ if (it.second->GetObjNum() == kInvalidObjNum)
it.second.Leak();
}
}
@@ -75,7 +75,8 @@
for (const auto& it : locker) {
if (!pdfium::Contains(*pVisited, it.second.Get())) {
std::set<const CPDF_Object*> visited(*pVisited);
- if (auto obj = it.second->CloneNonCyclic(bDirect, &visited))
+ auto obj = it.second->CloneNonCyclic(bDirect, &visited);
+ if (obj)
pCopy->m_Map.insert(std::make_pair(it.first, std::move(obj)));
}
}
diff --git a/core/fpdfapi/parser/cpdf_dictionary.h b/core/fpdfapi/parser/cpdf_dictionary.h
index a7d303f..d8ba1c0 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.h
+++ b/core/fpdfapi/parser/cpdf_dictionary.h
@@ -22,6 +22,8 @@
class CPDF_IndirectObjectHolder;
+// Dictionaries never contain nullptr for valid keys, but some of the methods
+// will return nullptr to indicate non-existent keys.
class CPDF_Dictionary final : public CPDF_Object {
public:
using const_iterator =
@@ -77,9 +79,10 @@
std::vector<ByteString> GetKeys() const;
// Creates a new object owned by the dictionary and returns an unowned
- // pointer to it. Prefer using these templates over calls to SetFor(),
- // since by creating a new object with no previous references, they ensure
- // cycles can not be introduced.
+ // pointer to it. Invalidates iterators for the element with the key |key|.
+ // Prefer using these templates over calls to SetFor(), since by creating
+ // a new object with no previous references, they ensure cycles can not be
+ // introduced.
template <typename T, typename... Args>
typename std::enable_if<!CanInternStrings<T>::value, T*>::type SetNewFor(
const ByteString& key,
@@ -95,14 +98,15 @@
key, pdfium::MakeRetain<T>(m_pPool, std::forward<Args>(args)...)));
}
+ // If |pObj| is null, then |key| is erased from the map. Otherwise, takes
+ // ownership of |pObj|, returns an unowned pointer to it. Invalidates
+ // iterators for the element with the key |key|.
+ CPDF_Object* SetFor(const ByteString& key, RetainPtr<CPDF_Object> pObj);
+
// Convenience functions to convert native objects to array form.
void SetRectFor(const ByteString& key, const CFX_FloatRect& rect);
void SetMatrixFor(const ByteString& key, const CFX_Matrix& matrix);
- // Set* functions invalidate iterators for the element with the key |key|.
- // Takes ownership of |pObj|, returns an unowned pointer to it.
- CPDF_Object* SetFor(const ByteString& key, RetainPtr<CPDF_Object> pObj);
-
void ConvertToIndirectObjectFor(const ByteString& key,
CPDF_IndirectObjectHolder* pHolder);