[yaml2obj] Move redundant statements into a separate static function

[jhenderson]

tools/yaml2obj/yaml2elf.cpp

229

"check" implies to me that it's just validating something. However, this function does more than that (it sets the Index variable too). I'd suggest renaming the function convertSectionIndex or similar.

230

SecField to me implies the name of a section field, e.g. "sh_link". I'd suggest renaming SecField to IndexSrc or similar, and Index to IndexDest. The two are closely related, so having similar names makes sense to me.

280

This isn't a section index lookup. It's a symbol lookup. Note that the message is different.

[jhenderson]

tools/yaml2obj/yaml2elf.cpp

280

By the way, were there no test failures because of this? If there were none, I'd suggest adding a test for this particular case, again in a separate change.

[Higuoxing]

tools/yaml2obj/yaml2elf.cpp

280

Sure, I'll add one.

[Higuoxing]

tools/yaml2obj/yaml2elf.cpp

280

I noticed this problem, because they are all of type NameToIdxMap.
I did not noticed the error hints are different, I'll fix them, thanks for your reviewing

[jhenderson]

tools/yaml2obj/yaml2elf.cpp

239

As this is only used the once, I don't think that it is useful to pull this case out into a separate function. I could be persuaded otherwise though.

[Higuoxing]

I just want to make the logic inside initSectionHeaders simple. I could wait until there are two places that needs simplify ...

Thank you @jhenderson, for your kind reviewing :)

[jhenderson]

LGTM.

tools/yaml2obj/yaml2elf.cpp

for (const auto &YamlPhdr : Doc.ProgramHeaders) {
     Phdr.p_type = YamlPhdr.Type;
     Phdr.p_flags = YamlPhdr.Flags;
     Phdr.p_vaddr = YamlPhdr.VAddr;
     Phdr.p_paddr = YamlPhdr.PAddr;
     PHeaders.push_back(Phdr);
   }
 }
 
+static bool convertSectionIndex(NameToIdxMap &SN2I, StringRef SecName,
+                                StringRef IndexSrc, unsigned &IndexDest) {
+  if (SN2I.lookup(IndexSrc, IndexDest) && !to_integer(IndexSrc, IndexDest)) {
+    WithColor::error() << "Unknown section referenced: '" << IndexSrc
+                       << "' at YAML section '" << SecName << "'.\n";
+    return false;
+  }
+  return true;
+}
+
 template <class ELFT>
 bool ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
                                         ContiguousBlobAccumulator &CBA) {
   // Ensure SHN_UNDEF entry is present. An all-zero section header is a
   // valid SHN_UNDEF entry since SHT_NULL == 0.
   Elf_Shdr SHeader;
   zero(SHeader);
   SHeaders.push_back(SHeader);
 
   for (const auto &Sec : Doc.Sections) {
     zero(SHeader);
     SHeader.sh_name = DotShStrtab.getOffset(Sec->Name);
     SHeader.sh_type = Sec->Type;
     SHeader.sh_flags = Sec->Flags;
     SHeader.sh_addr = Sec->Address;
     SHeader.sh_addralign = Sec->AddressAlign;
 
     if (!Sec->Link.empty()) {
       unsigned Index;
-      if (SN2I.lookup(Sec->Link, Index) && !to_integer(Sec->Link, Index)) {
-        WithColor::error() << "Unknown section referenced: '" << Sec->Link
-                           << "' at YAML section '" << Sec->Name << "'.\n";
+      if (!convertSectionIndex(SN2I, Sec->Name, Sec->Link, Index))
         return false;
-      }
       SHeader.sh_link = Index;
     }
 
     if (auto S = dyn_cast<ELFYAML::RawContentSection>(Sec.get()))
       writeSectionContent(SHeader, *S, CBA);
     else if (auto S = dyn_cast<ELFYAML::RelocationSection>(Sec.get())) {
       if (S->Link.empty())
         // For relocation section set link to .symtab by default.
         SHeader.sh_link = getDotSymTabSecNo();
 
       unsigned Index;
-      if (SN2I.lookup(S->Info, Index) && !to_integer(S->Info, Index)) {
-        WithColor::error() << "Unknown section referenced: '" << S->Info
-                           << "' at YAML section '" << S->Name << "'.\n";
+      if (!convertSectionIndex(SN2I, S->Name, S->Info, Index))
         return false;
-      }
       SHeader.sh_info = Index;
-
       if (!writeSectionContent(SHeader, *S, CBA))
         return false;
     } else if (auto S = dyn_cast<ELFYAML::Group>(Sec.get())) {
       unsigned SymIdx;
       if (SymN2I.lookup(S->Info, SymIdx) && !to_integer(S->Info, SymIdx)) {
         WithColor::error() << "Unknown symbol referenced: '" << S->Info
                            << "' at YAML section '" << S->Name << "'.\n";
         return false;
       }
       SHeader.sh_info = SymIdx;
       if (!writeSectionContent(SHeader, *S, CBA))
         return false;
     } else if (auto S = dyn_cast<ELFYAML::MipsABIFlags>(Sec.get())) {
       if (!writeSectionContent(SHeader, *S, CBA))
         return false;
bool ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
 
   auto &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
 
   for (auto member : Section.Members) {
     Elf_Word SIdx;
     unsigned int sectionIndex = 0;
     if (member.sectionNameOrType == "GRP_COMDAT")
       sectionIndex = llvm::ELF::GRP_COMDAT;
-    else if (SN2I.lookup(member.sectionNameOrType, sectionIndex) &&
-             !to_integer(member.sectionNameOrType, sectionIndex)) {
-      WithColor::error() << "Unknown section referenced: '"
-                         << member.sectionNameOrType << "' at YAML section' "
-                         << Section.Name << "\n";
+    else if (!convertSectionIndex(SN2I, Section.Name, member.sectionNameOrType,
+                                  sectionIndex))
       return false;
-    }
     SIdx = sectionIndex;
     OS.write((const char *)&SIdx, sizeof(SIdx));
   }
   return true;
 }
 
 template <class ELFT>
 bool ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,