Wrong usage of memcpy

[dkosmari]

In titledumper/source/main.c, line 371:

 memcpy(data, data + processed, length);

This should be a memmove(), not a memcpy(), since the addresses overlap. Fixing this stopped giving me corrupted file paths. This was found with valgrind.

[dimok789]

No this is correct as is. memcpy copies byte after byte incrementing the pointer. It is a valid usage like this. It can not overwrite something due to "overlapping" as the data pointer from which a byte is read is always behind the destination pointer. A memmove is not required and can not be a solution to whatever problem you are having. Besides I have no corruption when receiving files or any data and many people already would have reported something if it did corrupt because it is one main functionality of the receiver. Something other than that is wrong for you.

[dkosmari]

You're wrong. The standard (quoted from ISO-IEC 9899:2011, 7.24.2.1) says:

The memcpy function copies n characters from the object pointed to by s2 into the
object pointed to by s1. If copying takes place between objects that overlap, the behavior
is undefined.

Surely you understand what "undefined behavior" means.

EDIT:

Two examples:

Here I added a printf to warn when trying to write to a NULL file pointer instead of silently ignoring it. See what happened to the file name?
http://i.imgur.com/fhnx0Ey.png

And here's another, that's even worse because the file was created, but with the wrong path/name.
http://i.imgur.com/HI2gaww.png

Those are more likely to occur when resuming the dump; skipping existing files seems to cause more incorrect reshuffling of the receive buffer. Chances are, people are not reporting it because it happens silently, and a re-run of the dump will possibly create the file properly the next time around; so the incorrect file will just be ignored.

Sometimes I'll also get a bunch of "Unknown TAG" messages, and other times it'll just get stuck after writing a file; often it's the rpx file that freezes the process. Some times the process will end before the game is completely dumped; and if I try to resume from that, I'll get even more garbage files. When the process gets stuck, the Wii U side also seems to get stuck, and the system menu icons won't load, until I forcefully kill the titledumper process.

EDIT 2:

All those issues disappear when I fix that line to use memmove. I think people have been reported this bug, but due to its random nature they think it got fixed, and you have been associating it with unrelated fixes. Bugs #2, #3, #4, #8, #9 all sound very familiar to what I'm seeing.

[dimok789]

It is undefined because the output depends on the input. If you know the exact implementation of the memcpy then it is done correct.

Alright. I get your point though. It depends on the implementation of the OS and we can't just depend on that. If the OS implementation makes it different than the usual *dst++ = *src++ implementation than it is possible that the behaviour is different. It could of course be an implementation where it decrements the pointer and that would fail in that case.

Can you make a pull request?

EDIT:
The issues #2, #3, #4 were not related to this in any way as those were issues on the WiiU side and not the host side.

[dimok789]

Pull request is merged.