<div dir="ltr">Looks great!</div><br><div class="gmail_quote"><div dir="ltr">On Thu, Aug 4, 2016 at 3:02 PM Kevin Enderby via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: enderby<br>
Date: Thu Aug 4 16:54:19 2016<br>
New Revision: 277776<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=277776&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=277776&view=rev</a><br>
Log:<br>
Clean up the logic of the Archive::Child::Child() with an assert to know Err is not a nullptr<br>
when we are pointed at real data.<br>
<br>
David Blaikie pointed out some odd logic in the case the Err value was a nullptr and<br>
Lang Hames suggested it could be cleaned it up with an assert to know that Err is<br>
not a nullptr when we are pointed at real data. As only in the case of constructing<br>
the sentinel value by pointing it at null data is Err is permitted to be a nullptr,<br>
since no error could occur in that case.<br>
<br>
With this change the testing for “if (Err)” is removed from the constructor’s logic<br>
and *Err is used directly without any check after the assert().<br>
<br>
Modified:<br>
llvm/trunk/lib/Object/Archive.cpp<br>
<br>
Modified: llvm/trunk/lib/Object/Archive.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/Archive.cpp?rev=277776&r1=277775&r2=277776&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/Archive.cpp?rev=277776&r1=277775&r2=277776&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Object/Archive.cpp (original)<br>
+++ llvm/trunk/lib/Object/Archive.cpp Thu Aug 4 16:54:19 2016<br>
@@ -310,27 +310,32 @@ Archive::Child::Child(const Archive *Par<br>
(Start - Parent->getData().data()), Err) {<br>
if (!Start)<br>
return;<br>
+<br>
+ // If we are pointed to real data, Start is not a nullptr, then there must be<br>
+ // a non-null Err pointer available to report malformed data on. Only in<br>
+ // the case sentinel value is being constructed is Err is permitted to be a<br>
+ // nullptr.<br>
+ assert(Err && "Err can't be nullptr if Start is not a nullptr");<br>
+<br>
ErrorAsOutParameter ErrAsOutParam(Err);<br>
<br>
- // If there was an error in the construction of the Header and we were passed<br>
- // Err that is not nullptr then just return with the error now set.<br>
- if (Err && *Err)<br>
+ // If there was an error in the construction of the Header<br>
+ // then just return with the error now set.<br>
+ if (*Err)<br>
return;<br>
<br>
uint64_t Size = Header.getSizeOf();<br>
Data = StringRef(Start, Size);<br>
Expected<bool> isThinOrErr = isThinMember();<br>
if (!isThinOrErr) {<br>
- if (Err)<br>
- *Err = isThinOrErr.takeError();<br>
+ *Err = isThinOrErr.takeError();<br>
return;<br>
}<br>
bool isThin = isThinOrErr.get();<br>
if (!isThin) {<br>
Expected<uint64_t> MemberSize = getRawSize();<br>
if (!MemberSize) {<br>
- if (Err)<br>
- *Err = MemberSize.takeError();<br>
+ *Err = MemberSize.takeError();<br>
return;<br>
}<br>
Size += MemberSize.get();<br>
@@ -342,26 +347,23 @@ Archive::Child::Child(const Archive *Par<br>
// Don't include attached name.<br>
Expected<StringRef> NameOrErr = getRawName();<br>
if (!NameOrErr){<br>
- if (Err)<br>
- *Err = NameOrErr.takeError();<br>
+ *Err = NameOrErr.takeError();<br>
return;<br>
}<br>
StringRef Name = NameOrErr.get();<br>
if (Name.startswith("#1/")) {<br>
uint64_t NameSize;<br>
if (Name.substr(3).rtrim(' ').getAsInteger(10, NameSize)) {<br>
- if (Err) {<br>
- std::string Buf;<br>
- raw_string_ostream OS(Buf);<br>
- OS.write_escaped(Name.substr(3).rtrim(' '));<br>
- OS.flush();<br>
- uint64_t Offset = Start - Parent->getData().data();<br>
- *Err = malformedError("long name length characters after the #1/ are "<br>
- "not all decimal numbers: '" + Buf + "' for "<br>
- "archive member header at offset " +<br>
- Twine(Offset));<br>
- return;<br>
- }<br>
+ std::string Buf;<br>
+ raw_string_ostream OS(Buf);<br>
+ OS.write_escaped(Name.substr(3).rtrim(' '));<br>
+ OS.flush();<br>
+ uint64_t Offset = Start - Parent->getData().data();<br>
+ *Err = malformedError("long name length characters after the #1/ are "<br>
+ "not all decimal numbers: '" + Buf + "' for "<br>
+ "archive member header at offset " +<br>
+ Twine(Offset));<br>
+ return;<br>
}<br>
StartOfFile += NameSize;<br>
}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>