JANITORIAL: DEVTOOLS: add missing special functions to file wrapper classes

[meekee7]

The devtools contain many stdio.h file IO wrappers. Most of them lack special member functions declarations, making their implementation hazardous. The lack of a destructor means potential file handle leaks. The fact that copy and move are implicitly allowed means potential file handle duplication, double file handle closures and file handle use after closure.

Missing destructors are added, copying and moving is forbidden for all File classes.
In all cases, the close implementation is reentrant-safe, meaning that a file closed manually can still be safely re-closed in the destructor.

MemFile from create_xeen has a different situation. That class is used as a return value, meaning that it needs copy and move operations defined. Nonetheless the implicit functions were unsafe: C++11 does not guarantee the copy elision - that would require C++17. Thus there was unsafe copying and potential double-deletes masked by compiler optimizations.

[sev-]

I am not sure what actual (not theoretical) problem you are trying to solve here.

Also, regardless of the reasoning, we still require following our Code Formatting Guidelines.

[sev-]

@meekee7 any updates?

[meekee7]

I have applied clang-format to the relevant parts.

[meekee7]

The classes in question were not following the C++ rule of five.

In create_xeen, MemFile is returned by value in CCArchive::getMember. C++11 does not guarantee copy elision, meaning that copy/move operations could be used. The implicit copy/move operations are wrong in this case and could lead to use-after-delete and double-delete of the memory buffer. Explicit definitions are needed.

For the other file handle wrapper classes, there are luckily no active hazards in their current usage. Nonetheless they are a code accident waiting to happen, just one change or refactoring away from creating a problem that is easily missed.
The implementation set a bad example; and the frequent copy pasting of those classes indicates that the bad example has proliferated.

[sev-]

We normally write and forget the tools. I am not great fan of writing code just for the sake of writing it.

How was the MemFile copy assignment operator implementation tested?

[sev-]

I would suggest looking at PVS-Studio and Coverity; we have a significant amount of real issues there.

If you could help here, you would be more than welcome. Here is our guide: https://wiki.scummvm.org/index.php?title=HOWTO-Static_Analysis_Tools