Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#133 Fix memory block alignment in RomBanner for 4-byte wchar_t

closed-fixed
nobody
None
5
2011-07-23
2011-07-05
rogerman
No

The RomBanner struct requires UTF-16 strings for the six different ROM titles. Currently, we use wchar_t buffers to define the memory blocks where the title strings are supposed to go. Using wchar_t is fine for compilers that assume a 2-byte wchar_t. But compilers that use a 4-byte wchar_t will misalign the memory blocks.

This patch removes the assumption of a 2-byte wchar_t defining the size of the memory blocks. If the compiler uses a 4-byte wchar_t, then the port code becomes responsible for properly reencoding the string data as necessary.

Discussion

  • zeromus
    zeromus
    2011-07-05

    I dont like this patch. It looks buggy, and adding broken utf-* cruft into the codebase displeases me.
    1. if we can't use wchar_t to represent utf-16, then change it all to use u16 all the time in the NDS header code.
    2. when a utf-16 string from the header needs to be printed, then someone needs to convert it to utf-32 the right way. we have some code in xstring.cpp which converts from std::string to std::wstring. but it needs to be refactored (or more code needs to be ripped off from unicode.org) to convert from utf-16 to utf-32. we should use std::basic_string<u16> to pass around header-related strings in the emulator internal since std::wstring is useless. so we should have a function to convert a std::basic_string<u16> to a std::wstring or a std::string so that frontends can display it.

    basically, i hate all this foolishness, but if we're going to do it, we're going to do it _right_. if you want to fix memory corruption in your sizeof(wchar_t)==4 system then just change the header to use u16 everywhere and bypass the code that uses it for printing/display with a check, rather than code like this

    + wcscpy((wchar_t*)titles[i],L"None");

    which doesnt even look right. (titles contains u16 or utf-16, not wchar_t on your platform)

     
  • rogerman
    rogerman
    2011-07-07

    I've submitted a new patch that changes all the wchar_t in RomBanner titles to u16 for all ports. It took me a while to research the dependencies on titles, and to make sure I didn't accidentally break something in the Windows port.

    As for the wcscpy() deal in the RomBanner constructor, it's an arbitrary default init value that we don't actually use in practice, so I removed it. If a particular port wants to know what to display for the titles when a ROM isn't loaded, there are plenty of ways to figure this out. Or they can just check the titles for an empty string, which the constructor inits the titles to before the wcscpy() call.

    For CHEATS::save(), I removed the dependency on using titles. The ROM name we write to file is also an arbitrary value which we don't actually reference in practice. I changed it to use gameInfo.ROMname, since we're already using gameInfo.ROMserial.

     
    • status: open --> closed-fixed
     
  • Applied, thanks.