Fix for potential crash when splitting or merging paks

[ceeac]

When splitting or merging paks, makeobj may crash due to a use-after-scope bug. This patch fixes the bug.

[Dwachs]

What is the point in deleting "using std::string" ? It makes the patch hard to read.

[ceeac]

IMO the code is easier to read with "using std::string" removed because having a std:: prefix  provides the information that whatever comes after is part of the Standard Library. But I do not have a strong opinion on this. I attached an updated patch without the "using std::string" changes.

[Dwachs]

I do not see the crash from the patch. Which variable is used after scope? I see that name is changed inside these methods but it is not used afterwards. Still confused.

[ceeac]

In root_writer_t::copy, line 290 in current trunk there is this piece of code:

Code Select Expand


if (outfp == NULL) {
name = find.complete(name, "pak").c_str();
outfp = fopen(name, "wb");
}

find.complete returns by value, and a pointer to the temporary value gets assigned to name, and name gets used afterwards. The same is true for root_writer_t::uncopy (line 361).

[prissi]

The code is weird anyway. Why first delete the content of an existing file by open it for writing, then closing it, delete it and to write a temporary file only to finally rename it to the name of the file it deleted before. That logic is broken indeed.

Since the file does not need to be closed after opening and removing all existing content, the use of name after it could have been fixed with

Code Select Expand


- name = find.complete(name, "pak").c_str();
- outfp = fopen(name, "wb");
+ outfp = fopen(find.complete(name, "pak").c_str(), "wb");

see diff

[Dwachs]

@ceeac thanks for the explanation (and spotting this)