[PATCH] D144872: [AIX] Align the content of an xcoff object file which has auxiliary header in big archive.

[Digger Lin via Phabricator via llvm-commits]

DiggerLin added inline comments.


================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:837
+        if (Error Err = SetNextSymFile(Buf, M->MemberName))
+          return std::move(Err);
+
----------------
jhenderson wrote:
> DiggerLin wrote:
> > jhenderson wrote:
> > > Test case needed.
> > we just refactor the code here, not adding a new functionality here, I do not think we need a new test case here. 
> Although I agree that the code to load a symbolic file is just a refactoring of existing code, this use of that code is a new call site, where an error needs checking in a test. If there's an existing test that hits this specific piece of code, then that's fine, but if there isn't, it needs a new test. Otherwise, there would be no test that shows that the error is properly handled in this case.
> 
> Same goes below.
there is existing test for the code when you implement the https://reviews.llvm.org/D88288
https://github.com/llvm/llvm-project/blob/main/llvm/test/Object/archive-malformed-object.test#L13
https://github.com/llvm/llvm-project/blob/main/llvm/test/Object/archive-malformed-object.test#L19


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144872