Replace "bool ok" style with early returns.

Stephen pointed out that he doesn't like the "bool ok" style and prefers
the early return style during an earlier code review. I agree, but was
using this style to match existing code.

This CL switches a number of methods over to the early return style.

BUG=None


Review URL: http://codereview.chromium.org/8499034

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@109137 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 72 insertions, 89 deletions

diff --git a/courgette/disassembler_elf_32_x86.cc b/courgette/disassembler_elf_32_x86.cc
index 5f3ba95..871cdb7 100644
--- a/courgette/disassembler_elf_32_x86.cc
+++ b/courgette/disassembler_elf_32_x86.cc
@@ -255,19 +255,17 @@ CheckBool DisassemblerElf32X86::RVAsToOffsets(std::vector<RVA>* rvas,
 }
 
 CheckBool DisassemblerElf32X86::ParseFile(AssemblyProgram* program) {
-  bool ok = true;
-
   // Walk all the bytes in the file, whether or not in a section.
   uint32 file_offset = 0;
 
   std::vector<size_t> abs_offsets;
   std::vector<size_t> rel_offsets;
 
-  if (ok)
-    ok = RVAsToOffsets(&abs32_locations_, &abs_offsets);
+  if (!RVAsToOffsets(&abs32_locations_, &abs_offsets))
+    return false;
 
-  if (ok)
-    ok = RVAsToOffsets(&rel32_locations_, &rel_offsets);
+  if (!RVAsToOffsets(&rel32_locations_, &rel_offsets))
+    return false;
 
   std::vector<size_t>::iterator current_abs_offset = abs_offsets.begin();
   std::vector<size_t>::iterator current_rel_offset = rel_offsets.begin();
@@ -276,34 +274,30 @@ CheckBool DisassemblerElf32X86::ParseFile(AssemblyProgram* program) {
   std::vector<size_t>::iterator end_rel_offset = rel_offsets.end();
 
   for (int section_id = 0;
-       ok && (section_id < SectionHeaderCount());
+       section_id < SectionHeaderCount();
        section_id++) {
 
     const Elf32_Shdr *section_header = SectionHeader(section_id);
 
-    if (ok) {
-      ok = ParseSimpleRegion(file_offset,
-                             section_header->sh_offset,
-                             program);
-      file_offset = section_header->sh_offset;
-    }
+    if (!ParseSimpleRegion(file_offset,
+                           section_header->sh_offset,
+                           program))
+      return false;
+    file_offset = section_header->sh_offset;
 
     switch (section_header->sh_type) {
       case SHT_REL:
-        if (ok) {
-          ok = ParseRelocationSection(section_header, program);
-          file_offset = section_header->sh_offset + section_header->sh_size;
-        }
+        if (!ParseRelocationSection(section_header, program))
+          return false;
+        file_offset = section_header->sh_offset + section_header->sh_size;
         break;
       case SHT_PROGBITS:
-        if (ok) {
-          ok = ParseProgbitsSection(section_header,
-                                    &current_abs_offset, end_abs_offset,
-                                    &current_rel_offset, end_rel_offset,
-                                    program);
-          file_offset = section_header->sh_offset + section_header->sh_size;
-        }
-
+        if (!ParseProgbitsSection(section_header,
+                                  &current_abs_offset, end_abs_offset,
+                                  &current_rel_offset, end_rel_offset,
+                                  program))
+          return false;
+        file_offset = section_header->sh_offset + section_header->sh_size;
         break;
       default:
         break;
@@ -311,16 +305,13 @@ CheckBool DisassemblerElf32X86::ParseFile(AssemblyProgram* program) {
   }
 
   // Rest of the file past the last section
-  if (ok) {
-    ok = ParseSimpleRegion(file_offset,
-                           length(),
-                           program);
-  }
+  if (!ParseSimpleRegion(file_offset,
+                         length(),
+                         program))
+    return false;
 
   // Make certain we consume all of the relocations as expected
-  ok = ok && (current_abs_offset == end_abs_offset);
-
-  return ok;
+  return (current_abs_offset == end_abs_offset);
 }
 
 CheckBool DisassemblerElf32X86::ParseRelocationSection(
@@ -342,7 +333,6 @@ CheckBool DisassemblerElf32X86::ParseRelocationSection(
   // all of our R_386_RELATIVE entries in the expected order followed
   // by assorted other entries we can't use special handling for.
 
-  bool ok = true;
   bool match = true;
 
   // Walk all the bytes in the section, matching relocation table or not
@@ -370,15 +360,12 @@ CheckBool DisassemblerElf32X86::ParseRelocationSection(
 
   if (match) {
     // Skip over relocation tables
-    ok = program->EmitElfRelocationInstruction();
+    if (!program->EmitElfRelocationInstruction())
+      return false;
     file_offset += sizeof(Elf32_Rel) * abs32_locations_.size();
   }
 
-  if (ok) {
-    ok = ParseSimpleRegion(file_offset, section_end, program);
-  }
-
-  return ok;
+  return ParseSimpleRegion(file_offset, section_end, program);
 }
 
 CheckBool DisassemblerElf32X86::ParseProgbitsSection(
@@ -389,22 +376,20 @@ CheckBool DisassemblerElf32X86::ParseProgbitsSection(
     std::vector<size_t>::iterator end_rel_offset,
     AssemblyProgram* program) {
 
-  bool ok = true;
-
   // Walk all the bytes in the file, whether or not in a section.
   size_t file_offset = section_header->sh_offset;
   size_t section_end = section_header->sh_offset + section_header->sh_size;
 
   Elf32_Addr origin = section_header->sh_addr;
   size_t origin_offset = section_header->sh_offset;
-  ok = program->EmitOriginInstruction(origin);
+  if (!program->EmitOriginInstruction(origin))
+    return false;
 
-  while (ok && file_offset < section_end) {
+  while (file_offset < section_end) {
 
     if (*current_abs_offset != end_abs_offset &&
-        file_offset > **current_abs_offset) {
-      ok = false;
-    }
+        file_offset > **current_abs_offset)
+      return false;
 
     while (*current_rel_offset != end_rel_offset &&
            file_offset > **current_rel_offset) {
@@ -424,28 +409,28 @@ CheckBool DisassemblerElf32X86::ParseProgbitsSection(
         next_relocation > (**current_rel_offset + 3))
       next_relocation = **current_rel_offset;
 
-    if (ok && (next_relocation > file_offset)) {
-      ok = ParseSimpleRegion(file_offset, next_relocation, program);
+    if (next_relocation > file_offset) {
+      if (!ParseSimpleRegion(file_offset, next_relocation, program))
+        return false;
 
       file_offset = next_relocation;
       continue;
     }
 
-    if (ok &&
-        *current_abs_offset != end_abs_offset &&
+    if (*current_abs_offset != end_abs_offset &&
         file_offset == **current_abs_offset) {
 
       const uint8* p = OffsetToPointer(file_offset);
       RVA target_rva = Read32LittleEndian(p);
 
-      ok = program->EmitAbs32(program->FindOrMakeAbs32Label(target_rva));
+      if (!program->EmitAbs32(program->FindOrMakeAbs32Label(target_rva)))
+        return false;
       file_offset += sizeof(RVA);
       (*current_abs_offset)++;
       continue;
     }
 
-    if (ok &&
-        *current_rel_offset != end_rel_offset &&
+    if (*current_rel_offset != end_rel_offset &&
         file_offset == **current_rel_offset) {
 
       const uint8* p = OffsetToPointer(file_offset);
@@ -455,7 +440,8 @@ CheckBool DisassemblerElf32X86::ParseProgbitsSection(
       RVA target_rva = (RVA)(origin + (file_offset - origin_offset) +
                              4 + relative_target);
 
-      ok = program->EmitRel32(program->FindOrMakeRel32Label(target_rva));
+      if (!program->EmitRel32(program->FindOrMakeRel32Label(target_rva)))
+        return false;
       file_offset += sizeof(RVA);
       (*current_rel_offset)++;
       continue;
@@ -463,11 +449,7 @@ CheckBool DisassemblerElf32X86::ParseProgbitsSection(
   }
 
   // Rest of the section (if any)
-  if (ok) {
-    ok = ParseSimpleRegion(file_offset, section_end, program);
-  }
-
-  return ok;
+  return ParseSimpleRegion(file_offset, section_end, program);
 }
 
 CheckBool DisassemblerElf32X86::ParseSimpleRegion(
@@ -480,13 +462,13 @@ CheckBool DisassemblerElf32X86::ParseSimpleRegion(
 
   const uint8* p = start;
 
-  bool ok = true;
-  while (p < end && ok) {
-    ok = program->EmitByteInstruction(*p);
+  while (p < end) {
+    if (!program->EmitByteInstruction(*p))
+      return false;
     ++p;
   }
 
-  return ok;
+  return true;
 }
 
 CheckBool DisassemblerElf32X86::ParseAbs32Relocs() {
diff --git a/courgette/disassembler_win32_x86.cc b/courgette/disassembler_win32_x86.cc
index 39cb695..10d7e4b 100644
--- a/courgette/disassembler_win32_x86.cc
+++ b/courgette/disassembler_win32_x86.cc
@@ -336,10 +336,9 @@ std::string DisassemblerWin32X86::SectionName(const Section* section) {
 }
 
 CheckBool DisassemblerWin32X86::ParseFile(AssemblyProgram* program) {
-  bool ok = true;
   // Walk all the bytes in the file, whether or not in a section.
   uint32 file_offset = 0;
-  while (ok && file_offset < length()) {
+  while (file_offset < length()) {
     const Section* section = FindNextSection(file_offset);
     if (section == NULL) {
       // No more sections.  There should not be extra stuff following last
@@ -349,15 +348,16 @@ CheckBool DisassemblerWin32X86::ParseFile(AssemblyProgram* program) {
     }
     if (file_offset < section->file_offset_of_raw_data) {
       uint32 section_start_offset = section->file_offset_of_raw_data;
-      ok = ParseNonSectionFileRegion(file_offset, section_start_offset,
-                                     program);
+      if(!ParseNonSectionFileRegion(file_offset, section_start_offset,
+                                    program))
+        return false;
+
       file_offset = section_start_offset;
     }
-    if (ok) {
-      uint32 end = file_offset + section->size_of_raw_data;
-      ok = ParseFileRegion(section, file_offset, end, program);
-      file_offset = end;
-    }
+    uint32 end = file_offset + section->size_of_raw_data;
+    if (!ParseFileRegion(section, file_offset, end, program))
+      return false;
+    file_offset = end;
   }
 
 #if COURGETTE_HISTOGRAM_TARGETS
@@ -365,7 +365,7 @@ CheckBool DisassemblerWin32X86::ParseFile(AssemblyProgram* program) {
   HistogramTargets("rel32 relocs", rel32_target_rvas_);
 #endif
 
-  return ok;
+  return true;
 }
 
 bool DisassemblerWin32X86::ParseAbs32Relocs() {
@@ -523,13 +523,13 @@ CheckBool DisassemblerWin32X86::ParseNonSectionFileRegion(
 
   const uint8* p = start;
 
-  bool ok = true;
-  while (p < end && ok) {
-    ok = program->EmitByteInstruction(*p);
+  while (p < end) {
+    if (!program->EmitByteInstruction(*p))
+      return false;
     ++p;
   }
 
-  return ok;
+  return true;
 }
 
 CheckBool DisassemblerWin32X86::ParseFileRegion(
@@ -551,20 +551,20 @@ CheckBool DisassemblerWin32X86::ParseFileRegion(
   std::vector<RVA>::iterator rel32_pos = rel32_locations_.begin();
   std::vector<RVA>::iterator abs32_pos = abs32_locations_.begin();
 
-  bool ok = program->EmitOriginInstruction(start_rva);
+  if (!program->EmitOriginInstruction(start_rva))
+    return false;
 
   const uint8* p = start_pointer;
 
-  while (ok && p < end_pointer) {
+  while (p < end_pointer) {
     RVA current_rva = static_cast<RVA>(p - adjust_pointer_to_rva);
 
     // The base relocation table is usually in the .relocs section, but it could
     // actually be anywhere.  Make sure we skip it because we will regenerate it
     // during assembly.
     if (current_rva == relocs_start_rva) {
-      ok = program->EmitPeRelocsInstruction();
-      if (!ok)
-        break;
+      if (!program->EmitPeRelocsInstruction())
+        return false;
       uint32 relocs_size = base_relocation_table().size_;
       if (relocs_size) {
         p += relocs_size;
@@ -580,9 +580,8 @@ CheckBool DisassemblerWin32X86::ParseFileRegion(
       RVA target_rva = target_address - image_base();
       // TODO(sra): target could be Label+offset.  It is not clear how to guess
       // which it might be.  We assume offset==0.
-      ok = program->EmitAbs32(program->FindOrMakeAbs32Label(target_rva));
-      if (!ok)
-        break;
+      if (!program->EmitAbs32(program->FindOrMakeAbs32Label(target_rva)))
+        return false;
       p += 4;
       continue;
     }
@@ -592,7 +591,8 @@ CheckBool DisassemblerWin32X86::ParseFileRegion(
 
     if (rel32_pos != rel32_locations_.end() && *rel32_pos == current_rva) {
       RVA target_rva = current_rva + 4 + Read32LittleEndian(p);
-      ok = program->EmitRel32(program->FindOrMakeRel32Label(target_rva));
+      if (!program->EmitRel32(program->FindOrMakeRel32Label(target_rva)))
+        return false;
       p += 4;
       continue;
     }
@@ -606,11 +606,12 @@ CheckBool DisassemblerWin32X86::ParseFileRegion(
       }
     }
 
-    ok = program->EmitByteInstruction(*p);
+    if (!program->EmitByteInstruction(*p))
+      return false;
     p += 1;
   }
 
-  return ok;
+  return true;
 }
 
 #if COURGETTE_HISTOGRAM_TARGETS