[PATCH] D100651: [AIX] Support of Big archive (read)

[Digger Lin via Phabricator via llvm-commits]

DiggerLin added inline comments.


================
Comment at: llvm/include/llvm/Object/Archive.h:101
+  ArMemHdrType const *ArMemHdr;
   Archive const *Parent;
+};
----------------
both the ArchiveMemberHeader and the BigArchiveMemberHeader has the "Archive const *Parent;" , can the member put in the AbstractArchiveMemberHeader ?


================
Comment at: llvm/lib/Object/Archive.cpp:60
+    : Parent(Parent) {
   if (RawHeaderPtr == nullptr)
     return;
----------------
if the RawHeaderPtr == nullptr is true ,  the value of ArMemHdr will be random here.


================
Comment at: llvm/lib/Object/Archive.cpp:106
+    : Parent(Parent) {
+  if (RawHeaderPtr == nullptr)
+    return;
----------------
same problem as above.


================
Comment at: llvm/lib/Object/Archive.cpp:163
+  // but unused terminator '`\n' is after the name.
+  StringRef::size_type end = strtol(ArMemHdr->NameLen, NULL, 10);
+  return StringRef(ArMemHdr->Name, end);
----------------
the variable name "end" can not express the mean, maybe be "NameSize" or "NameLen" ?


================
Comment at: llvm/lib/Object/Archive.cpp:308-320
     std::string Buf;
     raw_string_ostream OS(Buf);
     OS.write_escaped(
-        StringRef(ArMemHdr->AccessMode, sizeof(ArMemHdr->AccessMode))
-            .rtrim(" "));
+        StringRef(ArMemHdr->Size, sizeof(ArMemHdr->Size)).rtrim(" "));
     OS.flush();
     uint64_t Offset =
         reinterpret_cast<const char *>(ArMemHdr) - Parent->getData().data();
----------------
there are several place use the almost the same code from line 308 to 320 . can we change these lines into a helper function?


================
Comment at: llvm/lib/Object/Archive.cpp:558
+    // Add name to found the real start
+    StartOfFile += Name.size() + Name.size() % 2;
+  }
----------------
change "Name.size() + Name.size() % 2"  to ((Name.size()+1) >>1 ) << 1 ?


================
Comment at: llvm/lib/Object/Archive.cpp:561
+
   if (Name.startswith("#1/")) {
     uint64_t NameSize;
----------------
the Name.startswith("#1/")) is not for the Archive::K_AIXBIG. what happen if the AIXBIG archive file which has member name begin with "#/" here ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100651/new/

https://reviews.llvm.org/D100651