[PATCH] D89949: [yaml2obj][obj2yaml] - Teach tools to work with regular archives.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 27 06:21:01 PDT 2020


grimar added inline comments.


================
Comment at: llvm/test/tools/obj2yaml/Archives/regular.yaml:1
+## Check how obj2yaml dumps regular archives.
+
----------------
jhenderson wrote:
> Possible additional test case: dumping the output when the header fields are empty, where such are "legal" (i.e. I think all but the name).
Done.

> i.e. I think all but the name

The `Size` value should be a valid integer to read the content. The name can be empty, but it`s the default value for it, so it is not dumped.


================
Comment at: llvm/test/tools/obj2yaml/Archives/regular.yaml:59
+    PaddingByte:  0x0A
+## All fields are explicitly set to values by default.
+  - Name:         ''
----------------
jhenderson wrote:
> I think this comment better explains what's going on?
I think so.


================
Comment at: llvm/tools/obj2yaml/archive2yaml.cpp:37
+        return createStringError(std::errc::illegal_byte_sequence,
+                                 "unable to read the header of a child");
+
----------------
jhenderson wrote:
> I think it might be useful to include the offset of the bad header here. What do you think?
> 
> Same comment probably applies for most other messages below.
Done.


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

https://reviews.llvm.org/D89949



More information about the llvm-commits mailing list