The class NonCopyable is, like the name says, supposed to disallow
copying. But should it allow moving?
For a long time, NonCopyable used to not allow moving. (It declared
a deleted copy constructor and assigment operator without declaring
a move constructor and assignment operator, making the compiler
implicitly delete the move constructor and assignment operator.)
That's fine if the classes that inherit from NonCopyable don't need
to be movable or if writing the move constructor and assignment
operator by hand is fine, but that's not the case for all classes,
as I discovered when I was working on the DirectoryBlob PR.
Because of that, I decided to make NonCopyable movable in c7602cc,
allowing me to use NonCopyable in DirectoryBlob.h. That was however
an unfortunate decision, because some of the classes that inherit
from NonCopyable have incorrect behavior when moved by default-
generated move constructors and assignment operators, and do not
explicitly delete the move constructors and assignment operators,
relying on NonCopyable being non-movable.
So what can we do about this? There are four solutions that I can
think of:
1. Make NonCopyable non-movable and tell DirectoryBlob to suck it.
2. Keep allowing moving NonCopyable, and expect that classes that
don't support moving will delete the move constructor and
assignment operator manually. Not only is this inconsistent
(having classes disallow copying one way and disallow moving
another way), but deleting the move constructor and assignment
operator manually is too easy to forget compared to how tricky
the resulting problems are.
3. Have one "MovableNonCopyable" and one "NonMovableNonCopyable".
It works, but it feels rather silly...
4. Don't have a NonCopyable class at all. Considering that deleting
the copy constructor and assignment operator only takes two lines
of code, I don't see much of a reason to keep NonCopyable. I
suppose that there was more of a point in having NonCopyable back
in the pre-C++11 days, when it wasn't possible to use "= delete".
I decided to go with the fourth one (like the commit title says).
The implementation of the commit is fairly straight-forward, though
I would like to point out that I skipped adding "= delete" lines
for classes whose only reason for being uncopyable is that they
contain uncopyable classes like File::IOFile and std::unique_ptr,
because the compiler makes such classes uncopyable automatically.
Seems like I was wrong that ANDI2R doesn't require a temporary register here.
There is *one* case when the mask won't fit in the ARM AND instruction:
mask = 0xFFFFFFFF
But let's just use MOV instead of AND here for this case...
Given a relatively recent proposal (P0657R0), which calls for deprecation of putting stuff into the global namespace when using C++ headers, this just futureproofs our code a little more.
Technically this is what we should have been doing initially, since an
implementation is allowed to not provide these types in the global
namespace and still be compliant.
ifstream::read() sets the failbit if trying to read over the end, which
means that (!input) would be hit for the 'last' block if it wasn't
exactly BSIZE (1024) bytes.
Makes it easier to turn off general IOS messages that can be
distracting (e.g. /dev/net/ssl being opened hundreds of time...)
without losing the ability to view WFS messages.
The opagent library was (incorrectly) marked as a dependency for "Core"
instead of "Common".
When linked with --as-needed, any symbols the linker can tell are not
used are discarded. As the link is done in command-line order, and the
Core library (and dependencies) are processed before Common, it would
link in Core, then opagent, but as at that point no opagent symbols are
used the whole opagent library would be discarded.
Moving the opagent library to be a dependency of Common fixes this, as
after the Common library is linked, there *are* opagent symbols used.
* Add missing Language setting loading/saving. This was added after the
original OnionConfig PR, which is why support for it was missing.
* Change MovieConfigLoader to reuse ConfigInfos. Less duplication.
* Extract MovieConfigLoader::Save into SaveToDTM. The DTM should use
the current config and not just the movie layer. This makes more
sense than just saving the movie layer, which may not always exist,
and also fixes a crash that would happen when creating a new
recording because the movie layer wouldn't exist in that case.
(Plus, having to get the loader from the layer and call ChangeDTM
on it manually is not very pretty.)
Settings that come from the SYSCONF are now included in Dolphin's
config system as part of the base layer. They are handled in a
special way compared to other settings to make sure they are only
loaded from and saved to the SYSCONF (to avoid different, possibly
contradicting sources of truth).
Must be 9 characters at most; otherwise the serial number will be
rejected by SDK libraries, as there is a check to ensure the string
length is strictly lower than 10.
The Config::AddLoadLayer functions call Load on the layer
explicitly, but Load is already called in the constructor,
so they'd cause the loader's Load function to be called twice,
which is potentially expensive considering we have to read an INI
from the host filesystem.
This commit removes the Config::AddLoadLayer functions because
they don't appear to be necessary.
On Windows, File::GetTempFilenameForAtomicWrite returns a path
somewhere in C:\Users\XXX\AppData\Local\Temp\{UUID here}\
in which all writes just fail.
Just use the SYSCONF path + ".tmp" for the temporary file name.
This makes the EGL interface select OpenGL|ES contexts over "desktop"
OpenGL ones.
Possibly not useful for anyone outside my own debugging, but you never
know
The spec says it should have an EXT not OES suffix, as it's enabled as
an interaction with GL_EXT_multi_draw_arrays.
On some drivers GetProcAddress() returns NULL, which causes the
GLExtensions init to fail
This 'happened' to work if GetProcAddress() doesn't return NULL on missing
functions (as allowed in EGL) - as the function appears to never be called so
this would not have been noticed.
Mesa also (incorrectly?) exports the EXT version, so this would all
happen to work there, but appears to be contrary to the spec.
This invalid prefix even ended up in the upstream khronos registry, the
issue was reported here:
https://github.com/KhronosGroup/OpenGL-Registry/issues/81
It turns out that the last byte of array entries isn't unused (as we
thought); instead, it looks like it's actually part of the main data,
and the length stored next to the name is in fact the length minus one.
Getting it wrong and always storing a null byte in there won't affect
most entries (since the last byte is zeroed most of the time), except:
- IPL.NIK: the length is stored in the last byte, and it must be kept.
- BT.DINF: u8 unknown[0x45] should be another Bluetooth device entry.
- Possibly other unknown affected entries.
It's a bit confusing to get a yes/no dialogue box without any indication
of what yes or no will do in this situation, so add a short explanatory
sentence.
Some code was calling more than one of these functions in a row
(in particular, FileUtil.cpp itself did it a lot...), which is
a waste since it's possible to call stat a single time and then
read all three values from the stat struct. This commit adds a
File::FileInfo class that calls stat once on construction and
then lets Exists/IsDirectory/GetSize be executed very quickly.
The performance improvement mostly matters for functions that
can be handling a lot of files, such as File::ScanDirectoryTree.
I've also done some cleanup in code that uses these functions.
For instance, some code had checks like !Exists() || !IsDirectory(),
which is functionally equivalent to !IsDirectory(), and some
code was using File::GetSize even though there was an IOFile
object that the code could call GetSize on.
It didn't work when there were non-ASCII characters
in the directories argument, but it worked fine with
non-ASCII characters in names of found files and folders.
c5fa470 made the extension check discard directories, but
only in the new code that currently only is used on Windows.
Let's add an equivalent check in the old code so that the
behavior is consistent across platforms.
This fixes the global-static fifo object causing infinite hangs in some
cases. Notably, failure to initialize a graphics backend would result in
BlockingLoop::Prepare being called but never executing Run(), leaving the
object in a bad state.
Pierre Bourdon