# Efficient binary patcher

I am currently working on a little binary patcher. My current code works fine but I do want my code to be optimized (fast) and as clean as possible. Could you all please help me review this?

IDE: Delphi 7 and XE7

const
OriginalByte: array [0 .. 55] of Byte = ($64,$69, $6E,$67, $20,$6D, $65,$20, $68,$69, $73,$00, $00,$00, $00,$27, $00,$00, $28,$00, $00,$00,
$21,$00, $00,$00, $63,$6F, $6D,$70, $75,$74, $65,$72, $2E,$00, $00,$00, $00,$00, $00,$00, $00,$00, $00,$00, $00,$00, $27,$00, $00,$28,
$00,$00, $00,$00);

BytetoWrite: array [0 .. 55] of Byte = ($63,$68, $6F,$67, $20,$6D, $65,$20, $68,$69, $73,$00, $00,$00, $00,$27, $00,$00, $28,$00, $00,$00,
$21,$00, $00,$00, $63,$6F, $6D,$70, $75,$74, $65,$72, $2E,$00, $00,$00, $00,$00, $00,$00, $00,$00, $00,$00, $00,$00, $27,$00, $00,$28,
$00,$00, $00,$06);

function CompareByteArrays(FirstArray, SecondArray: array of Byte): Boolean;

var
Counter: Integer;
begin

Result := True;
if Length(FirstArray) <> Length(SecondArray) then
begin
Result := False;
Exit;
end;

for Counter := Low(FirstArray) to High(FirstArray) do

begin
if (FirstArray[Counter]) <> (SecondArray[Counter]) then
begin
Result := False;
Exit;
end

end;

end;

procedure DoMyPatch();
var

i: Integer;
FileName: string;
input: TFileStream;

FileByteArray, ExtractedByteArray: array of Byte;

begin
FileName := 'Cute1.res';

try
except
on E: Exception do
begin
ShowMessage(E.Message);
Exit;
end
end;

input.Position := 0;
SetLength(FileByteArray, input.size);
for i := Low(FileByteArray) to High(FileByteArray) do
begin

ExtractedByteArray := Copy(FileByteArray, i, Length(OriginalByte));

if CompareByteArrays(ExtractedByteArray, OriginalByte) = True then
begin
// Begin Patch
input.Free;
try
Win32Check(CopyFile(PChar(FileName), PChar(ChangeFileExt(FileName,
'.BAK')), False));
except
on E: Exception do
begin
ShowMessage(E.Message);
Exit;
end
end;

try
except
on E: Exception do
begin
ShowMessage(E.Message);
Exit;
end
end;

input.Seek(i, SoFromBeginning);

input.Write(BytetoWrite[0], Length(BytetoWrite));

ShowMessage('Patch Success');
input.Free;
Exit;
end;
end;
ShowMessage('Patch Fail');

input.Free;

end;


Some notes:

1. add definition of the undeclared variables BytetoWrite, OriginalByte and how they are used/initialized
2. format your code for better readability. Left indentation is not aligned. Too many void blank lines
3. using

resource := CreateResource;
try
// do something
finally
FreeAndNil(resource);
end

pattern is easy to follow and it prevents unintended memory leaks nicely

4. for byte-to-byte memory compare in CompareByteArrays better use built-in CompareMem function instead of custom array traversal. In an old language runtime source codes this function was implemented in optimized assembly language
5. using Integer for file-offset is not very future-friendly idea considering that it is normal (for average end-user) today to work with video files in 4K resolution where single file size can be >4GB
6. using Memory Mapped Files for read-only file access will give you simpler code (whole file is an in-memory array) and faster I/O operations (on-demand reading managed by OS kernel code). This method can give you significant boost if the original file is big and the searched-pattern is located near the beginning of the file
7. the loop should end at High(FileByteArray) - Length(OriginalByte) (+/-1) as beyond this boundary the pattern can not be found anyway
8. you can get some speed by comparing against the original byte array just offset by the i index. Creation of the intermediate ExtractedByteArray would not be necessary (throw away, safe some memory allocation and memory copy CPU-clocks). Like this:

if CompareByteArrays(ExtractedByteArray, OriginalByte) = True then
if CompareMem(@FileByteArray[i], @OriginalByte[0], Length(OriginalByte)) = True then

assuming the i...i + Length(OriginalByte) - 1 boundaries are withing the range Low(FileByteArray)...High(FileByteArray) as mentioned in (7)

• thanks for your comments, and sorry i can't upvote you (not enough reputation) but i do have one question. if i use resource := CreateResource; try try // do something except raise exit; end finally FreeAndNil(resource); end the "finally" segment of the code doesn't get executed because of the "exit" command right? Commented Dec 30, 2014 at 16:22
• @Xor-el finally block will execute always (unless your application is crashing irrecoverably or the computer is going out of e-power). See Stack Overflow: Does the statements in the Finally block still execute in this piece of code? for some "proof" Commented Dec 30, 2014 at 16:57
• @Xor-el thanks for your accept/vote although it was probably too fast as some better answer may appear once the new year's eve holidays are over. Looking at your code after the edit it seems that the patcher is single-purpose so the (5) does not apply. Using (6) can give you significant boost if the original file is big and the searched-pattern is located near the beginning of the file. (7) the loop should end at High(FileByteArray) - Length(OriginalByte) (+/-1) otherwise you'll face the (random) access violation problem if the pattern is not there Commented Dec 30, 2014 at 18:36
• i don't think so because i have tested it with a pattern that does not exist and it works fine (it does not crash) and informs me that pattern was not found (patch fail). Commented Dec 30, 2014 at 21:24
• @xmomjr, nice habit i must say. Commented Dec 31, 2014 at 14:11