[llvm] r277776 - Clean up the logic of the Archive::Child::Child() with an assert to know Err is not a nullptr

[David Blaikie via llvm-commits]

Looks great!

On Thu, Aug 4, 2016 at 3:02 PM Kevin Enderby via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

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