Last small bit of refactoring.

Move the Win32X86Generator and Patcher classes to non-windows specific names since they can be reused untouched for Elf. Move them from one file to files with matching names while at it. Store the transformation kind on the generator class so that it can be know if it's PE or Elf. Unified the TransformationId and ExecutableType enums into a single enum used everywhere (defined in courgette.h since it's now part of the external API). BUG=chromium-os:22149 TEST=Unittests Review URL: http://codereview.chromium.org/8417045 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@108019 0039d316-1c4b-4281-b951-d872f2087c98
FairyWorld · Oct 31, 2011 · c5fc1df · c5fc1df
1 parent 6915c26
commit c5fc1df
Show file tree

Hide file tree

Showing 10 changed files with 35 additions and 38 deletions.
diff --git a/courgette/courgette.gyp b/courgette/courgette.gyp
@@ -37,8 +37,8 @@
       'simple_delta.h',
       'streams.cc',
       'streams.h',
-      'win32_x86_generator.h',
-      'win32_x86_patcher.h',
+      'patch_generator_x86_32.h',
+      'patcher_x86_32.h',
     ],
   },
   'targets': [

diff --git a/courgette/courgette.h b/courgette/courgette.h
@@ -51,10 +51,10 @@ enum Status {
 };
 
 // What type of executable is something
-// Generally corresponds to CourgettePatchFile::TransformationMethodId
+// This is part of the patch format. Never reuse an id number.
 enum ExecutableType {
-  UNKNOWN,
-  WIN32_X86
+  EXE_UNKNOWN = 0,
+  EXE_WIN_32_X86 = 1,
 };
 
 class SinkStream;

diff --git a/courgette/disassembler.cc b/courgette/disassembler.cc
@@ -50,7 +50,7 @@ Status DetectExecutableType(const void* buffer, size_t length,
   }
 
   // We failed to detect anything
-  *type = UNKNOWN;
+  *type = EXE_UNKNOWN;
   *detected_length = 0;
   return C_INPUT_NOT_RECOGNIZED;
 }

diff --git a/courgette/disassembler.h b/courgette/disassembler.h
@@ -21,7 +21,7 @@ class Disassembler {
  public:
   virtual ~Disassembler();
 
-  virtual ExecutableType kind() { return UNKNOWN; }
+  virtual ExecutableType kind() { return EXE_UNKNOWN; }
 
   // ok() may always be called but returns 'true' only after ParseHeader
   // succeeds.

diff --git a/courgette/disassembler_win32_x86.h b/courgette/disassembler_win32_x86.h
@@ -18,7 +18,7 @@ class DisassemblerWin32X86 : public Disassembler {
  public:
   explicit DisassemblerWin32X86(const void* start, size_t length);
 
-  virtual ExecutableType kind() { return WIN32_X86; }
+  virtual ExecutableType kind() { return EXE_WIN_32_X86; }
 
   // Returns 'true' if the buffer appears to point to a Windows 32 bit
   // executable, 'false' otherwise.  If ParseHeader() succeeds, other member

diff --git a/courgette/ensemble.h b/courgette/ensemble.h
@@ -133,12 +133,6 @@ struct CourgettePatchFile {
   static const uint32 kMagic = 'C' | ('o' << 8) | ('u' << 16);
 
   static const uint32 kVersion = 20110216;
-
-  // Transformation method IDs. These are embedded in generated files, so
-  // never remove or change an existing id.
-  enum TransformationMethodId {
-    T_COURGETTE_WIN32_X86 = 1,  // Windows 32 bit 'Portable Executable' x86.
-  };
 };
 
 // For any transform you would implement both a TransformationPatcher and a
@@ -208,7 +202,7 @@ class TransformationPatchGenerator {
   virtual ~TransformationPatchGenerator();
 
   // Returns the TransformationMethodId that identies this transformation.
-  virtual CourgettePatchFile::TransformationMethodId Kind() = 0;
+  virtual ExecutableType Kind() = 0;
 
   // Writes the parameters that will be passed to TransformationPatcher::Init.
   virtual Status WriteInitialParameters(SinkStream* parameter_stream) = 0;

diff --git a/courgette/ensemble_apply.cc b/courgette/ensemble_apply.cc
@@ -14,7 +14,7 @@
 #include "courgette/region.h"
 #include "courgette/streams.h"
 #include "courgette/simple_delta.h"
-#include "courgette/win32_x86_patcher.h"
+#include "courgette/patcher_x86_32.h"
 
 namespace courgette {
 
@@ -139,8 +139,8 @@ Status EnsemblePatchApplication::ReadInitialParameters(
 
     switch (kind)
     {
-      case CourgettePatchFile::T_COURGETTE_WIN32_X86:
-        patcher = new CourgetteWin32X86Patcher(base_region_);
+      case EXE_WIN_32_X86:
+        patcher = new PatcherX86_32(base_region_);
         break;
     }
 

diff --git a/courgette/ensemble_create.cc b/courgette/ensemble_create.cc
@@ -28,8 +28,8 @@
 #include "courgette/region.h"
 #include "courgette/simple_delta.h"
 
-#include "courgette/win32_x86_patcher.h"
-#include "courgette/win32_x86_generator.h"
+#include "courgette/patcher_x86_32.h"
+#include "courgette/patch_generator_x86_32.h"
 
 namespace courgette {
 
@@ -65,14 +65,15 @@ Status TransformationPatchGenerator::Reform(
 TransformationPatchGenerator* MakeGenerator(Element* old_element,
                                             Element* new_element) {
   switch (new_element->kind()) {
-    case UNKNOWN:
+    case EXE_UNKNOWN:
       break;
-    case WIN32_X86: {
+    case EXE_WIN_32_X86: {
       TransformationPatchGenerator* generator =
-          new CourgetteWin32X86PatchGenerator(
+          new PatchGeneratorX86_32(
               old_element,
               new_element,
-              new CourgetteWin32X86Patcher(old_element->region()));
+              new PatcherX86_32(old_element->region()),
+              EXE_WIN_32_X86);
       return generator;
     }
   }
@@ -240,7 +241,7 @@ Status GenerateEnsemblePatch(SourceStream* base,
     return C_STREAM_ERROR;
 
   for (size_t i = 0;  i < number_of_transformations;  ++i) {
-    CourgettePatchFile::TransformationMethodId kind = generators[i]->Kind();
+    ExecutableType kind = generators[i]->Kind();
     if (!tranformation_descriptions->WriteVarint32(kind))
       return C_STREAM_ERROR;
   }

diff --git a/courgette/win32_x86_generator.h → courgette/patch_generator_x86_32.h b/courgette/win32_x86_generator.h → courgette/patch_generator_x86_32.h
@@ -14,17 +14,17 @@
 
 namespace courgette {
 
-class CourgetteWin32X86PatchGenerator : public TransformationPatchGenerator {
+class PatchGeneratorX86_32 : public TransformationPatchGenerator {
  public:
-  CourgetteWin32X86PatchGenerator(Element* old_element,
+  PatchGeneratorX86_32(Element* old_element,
                                   Element* new_element,
-                                  CourgetteWin32X86Patcher* patcher)
-      : TransformationPatchGenerator(old_element, new_element, patcher) {
+                                  PatcherX86_32* patcher,
+                                  ExecutableType kind)
+      : TransformationPatchGenerator(old_element, new_element, patcher),
+        kind_(kind) {
   }
 
-  CourgettePatchFile::TransformationMethodId Kind() {
-    return CourgettePatchFile::T_COURGETTE_WIN32_X86;
-  }
+  virtual ExecutableType Kind() { return kind_; }
 
   Status WriteInitialParameters(SinkStream* parameter_stream) {
     if (!parameter_stream->WriteSizeVarint32(
@@ -124,9 +124,11 @@ class CourgetteWin32X86PatchGenerator : public TransformationPatchGenerator {
   }
 
  private:
-  ~CourgetteWin32X86PatchGenerator() { }
+  virtual ~PatchGeneratorX86_32() { }
+
+  ExecutableType kind_;
 
-  DISALLOW_COPY_AND_ASSIGN(CourgetteWin32X86PatchGenerator);
+  DISALLOW_COPY_AND_ASSIGN(PatchGeneratorX86_32);
 };
 
 }  // namespace courgette

diff --git a/courgette/win32_x86_patcher.h → courgette/patcher_x86_32.h b/courgette/win32_x86_patcher.h → courgette/patcher_x86_32.h
@@ -11,12 +11,12 @@
 
 namespace courgette {
 
-// CourgetteWin32X86Patcher is a TransformationPatcher for Windows 32-bit
+// Courgette32X86Patcher is a TransformationPatcher for Windows 32-bit
 // executables.
 //
-class CourgetteWin32X86Patcher : public TransformationPatcher {
+class PatcherX86_32 : public TransformationPatcher {
  public:
-  explicit CourgetteWin32X86Patcher(const Region& region)
+  explicit PatcherX86_32(const Region& region)
       : ensemble_region_(region) {
   }
 
@@ -86,7 +86,7 @@ class CourgetteWin32X86Patcher : public TransformationPatcher {
   uint32 base_offset_;
   uint32 base_length_;
 
-  DISALLOW_COPY_AND_ASSIGN(CourgetteWin32X86Patcher);
+  DISALLOW_COPY_AND_ASSIGN(PatcherX86_32);
 };
 
 }  // namespace