There is a rule of thumb with constructors, don't put constructors inside the try-except or try-finally block that frees them. Secondly, you should always use try-finally for freeing this not only avoids you having the two FreeAndNil in your code, but also frees on all other jumps such as Exit, Break or Continue. Once you use Finally you don't need to think about where you need to free anymore, because Finally will always be called
Always have your code as follows:
MyInstance := MyClass.Create;
try
...
finally
MyInstance.Free;
end;
The Create should never be in the same try-block that performs the free, because when a constructor fails, the memory is freed, but the result variable is in an undefined state (is btw true for any function, if a function throws an exception never touch the result). So if the Create fails in this construct, the exception will be raised and the try-finally block will never be executed and therefore the Free will not be called.
Therefore looking at the whole picture, Nimrals answer is the correct one.
Second:
if Assigned(IniFile) then
FreeAndNil(IniFile);
This is a completely useless construct. Free is actually not the destructor, it is just a helper function (this indentation is in the original rtl source code):
procedure TObject.Free;
begin
// the call via self avoids a warning
if self<>nil then
self.destroy;
end;
If you call free you don't need to check assigned, if you check assigned you can skip calling free and call destroy instead.
Lastly, personally I do not like FreeAndNil, because while it is intended to make your code more safe, it often does the complete opposite. When you use FreeAndNil, it will set the instance to nil, so when you call Free a second time on it, the check of the helper seen above will kick in, and it just jumps over it. So nice, no error on double free. But thats a big problem, because if you call Free twice, this is usually indicative of some assumptions not matching up, resulting in the state that you think you need to free but the data is already freed. This means that you have an error somewhere before. So by not crashing, and just swallowing it anyway, you miss that error and might not find that bug, until it crashes really hard.
So what would actually be better would be something like this:
procedure FreeAndFill(var instance: TObject);
begin
if not Assigned(Instance) then
raise EDoubleFreeError.Create;
instance.Destroy;
Instance := nil;
end;
Or alternatively, to set the instance not to nil, but to anohter invalid pointer that will segfault, e.g. -1 (highest possible address).
Because not crashing on error is the worst kind of error condition you can have