[PATCH] D114429: [objcopy][NFC] Add doc comments to the executeObjcopy* functions.
Alexey Lapshin via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 25 03:15:16 PST 2021
avl added inline comments.
================
Comment at: llvm/include/llvm/Object/ObjCopy/ELF/ELFObjcopy.h:41
+/// Applies the transformations described by \p Config and \p ELFConfig to
+/// \p In(FileFormat::ELF or FileFormat::Unspecified) and writes
+/// the result into \p Out.
----------------
avl wrote:
> jhenderson wrote:
> > I'm a little thrown by the "Unspecified" format. I'm guessing that's just because earlier logic will detect that the input buffer is an ELF file if no explicit input format is specified, and feed it through to here. If that's the case, we shouldn't be mentioning this case, since in both cases, we're working with an ELF file.
> The idea of these descriptions is to match with values which are in:
>
> ```
> struct CommonConfig {
> ...
> FileFormat InputFormat = FileFormat::Unspecified;
> ...
> };
>
> enum class FileFormat {
> Unspecified,
> ELF,
> Binary,
> IHex,
> };
>
> ```
> It would probably be better to make FileFormat more descriptive by adding values: COFF, MachO, Wasm instead of Unspecified; RawELF instead of Binary... But I think this refactoring should be done separately. The idea of current doc comments is to show connection of CommonConfig.InputFormat and corresponding executeObjcopyOn* functions.
Current logic is that Unspecified is used for all formats:
```
// FIXME: Currently, we ignore the target for non-binary/ihex formats
// explicitly specified by -I option (e.g. -Ielf32-x86-64) and guess the
// format by llvm::object::createBinary regardless of the option value.
Config.InputFormat = StringSwitch<FileFormat>(InputFormat)
.Case("binary", FileFormat::Binary)
.Case("ihex", FileFormat::IHex)
.Default(FileFormat::Unspecified);
```
So, How about that variant:
```
/// Applies the transformations described by \p Config and \p ELFConfig to
/// \p In, which must be an ELF object(FileFormat::Unspecified), and writes
/// the result into \p Out.
```
?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114429/new/
https://reviews.llvm.org/D114429
More information about the llvm-commits
mailing list