[2.0.3/Windows]-Issues With "SDL_GetPrefPath()"?

[2.0.3/Windows]-Issues With “SDL_GetPrefPath()” ?

Hi,

We are working on converting a word game from SDL1 to SDL2.
We are having some problems with “SDL_GetPrefPath()” on some Windows computers.
Sometimes the game’s saving of data fails for no apparent reason?

Microsoft Windows XP, Vista, 7, 8, 10 32Bit/64Bit desktops/notebooks:
http://16bitsoft.com/files/LettersFall-Retail/Alpha/LFRetail-A5.zip
(compiled EXE - no source code / runs 100% under Linux using WINE 1.7+)

We were hoping some people could test the game by doing the following:

  1. Run game on computer.
  2. Change some options.
  3. Exit game.
  4. Re-run game and see if changed options are same.
  5. Reply to this post with test data: O.S. version and if saving/loading of game data is functioning properly.

This is a closed-source Windows only project, but we can show here how we save game data:

Code:
//-------------------------------------------------------------------------------------------------
void Data::SaveHighScoresAndOptions(void)
{
char filename[256];
fstream fileStream;
char textBuffer[50];
char *base_path = SDL_GetPrefPath(“16BitSoftInc”, “LettersFall-Retail”);
char *pref_path = NULL;

if (base_path)
{
    pref_path = SDL_strdup(base_path);
    SDL_free(base_path);
}
else  return;

strcpy(filename, pref_path);//settings->GetUserConfigPath() );
strcat(filename, "LettersFall-Alpha4-Data");

fileStream.open (filename, fstream::out);
if (fileStream.is_open())
{
	sprintf(textBuffer, "%d", audio->MusicVolume);
	fileStream<<textBuffer;
	fileStream<<"\n";

	sprintf(textBuffer, "%d", audio->SoundVolume);
	fileStream<<textBuffer;
	fileStream<<"\n";

Any help would be appreciated, thanks!------------------------
JeZxLee
JessePalser <AT> GMail <DOT> com
16BitSoft Inc.
Video Game Design Studio
www.16BitSoft.com

Windows 7 Home Premium, 64 Bit - saving/loading works good.

Jonas Thiem wrote:

Well the way you’re using strcat just asks for a buffer overflow. So I’d say it’s not that suprising?

While I think the maximum path length of windows is around 256, of course it no longer is if you append stuff. Also you don’t seem to check pref_path entered by the user for a proper max length either. So you seem to be kinda asking for undefined behavior and crashes.
Hi,

That is interesting perspective.
I’ll increase the size and test again on system that failed previously.

Many thanks!------------------------
JeZxLee
JessePalser <AT> GMail <DOT> com
16BitSoft Inc.
Video Game Design Studio
www.16BitSoft.com

As Jonas mentioned, maybe you should be more careful with the string
lengths in general? Use strncat() and keep track of the length of the path
so you have a more specific reason why it fails if it does fail due to long
paths.

Jonny DOn Wed, Apr 22, 2015 at 11:18 AM, JeZ-l-Lee < JessePalserMailingLists at gmail.com> wrote:

Jonas Thiem wrote:

Well the way you’re using strcat just asks for a buffer overflow. So I’d
say it’s not that suprising?

While I think the maximum path length of windows is around 256, of course
it no longer is if you append stuff. Also you don’t seem to check pref_path
entered by the user for a proper max length either. So you seem to be kinda
asking for undefined behavior and crashes.

Hi,

That is interesting perspective.
I’ll increase the size and test again on system that failed previously.

Many thanks!


JeZxLee
JessePalser GMail com
16BitSoft Inc.
Video Game Design Studio
www.16BitSoft.com


SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

As Jonas mentioned, maybe you should be more careful with the string
lengths in general? Use strncat() and keep track of the length of the

strncat() is a bit painful to use, strlcat() (available on all platforms
via SDL_strlcat()) is usually better.
(similar for (SDL_)strlcpy() instead of strncpy())

path so you have a more specific reason why it fails if it does fail due
to long paths.

Jonny D

Cheers,
DanielOn 04/22/2015 05:28 PM, Jonathan Dearborn wrote:

Daniel Gibson writes:> On 04/22/2015 05:28 PM, Jonathan Dearborn wrote:

As Jonas mentioned, maybe you should be more careful with the string
lengths in general? Use strncat() and keep track of the length of the

strncat() is a bit painful to use, strlcat() (available on all platforms
via SDL_strlcat()) is usually better.
(similar for (SDL_)strlcpy() instead of strncpy())

This is so important it is worth repeating: Do not ever use strncat or
strncpy unless you actually want their peculiar behaviour. I know of 1
single use case for that behaviour. I have almost never seen either of
them used correctly, and I have seen many cases of it causing broken
code. I have seen quite competent and careful programmers ending up with
undefined behaviour after using them.

Like Daniel, I recommend the “l” versions, which actually do what you
expect.

eirik

Yep, that’s good advice. Functions that can potentially drop the null
terminator are more often dangerous than not, particularly when there are
safer alternatives in widespread use.

Jonny DOn Wed, Apr 22, 2015 at 3:25 PM, Eirik Byrkjeflot Anonsen <eirik at eirikba.org wrote:

Daniel Gibson writes:

On 04/22/2015 05:28 PM, Jonathan Dearborn wrote:

As Jonas mentioned, maybe you should be more careful with the string
lengths in general? Use strncat() and keep track of the length of the

strncat() is a bit painful to use, strlcat() (available on all platforms
via SDL_strlcat()) is usually better.
(similar for (SDL_)strlcpy() instead of strncpy())

This is so important it is worth repeating: Do not ever use strncat or
strncpy unless you actually want their peculiar behaviour. I know of 1
single use case for that behaviour. I have almost never seen either of
them used correctly, and I have seen many cases of it causing broken
code. I have seen quite competent and careful programmers ending up with
undefined behaviour after using them.

Like Daniel, I recommend the “l” versions, which actually do what you
expect.

eirik


SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

strcpy(filename, pref_path);//settings->GetUserConfigPath() );
strcat(filename, "LettersFall-Alpha4-Data");

fileStream.open (filename, fstream::out);

Is it possible you have a user with a Unicode character (something with
an accent?) in their pref path? These will be converted to UTF-8 so they
fit in a const char * and work with strcpy() etc, but Windows
fstream::open() will almost certainly fail with this string unless you
convert it it back to UTF-16 and use wfstream instead.

–ryan.

Ryan C. Gordon wrote:

Is it possible you have a user with a Unicode character (something with
an accent?) in their pref path? These will be converted to UTF-8 so they
fit in a const char * and work with strcpy() etc, but Windows
fstream::open() will almost certainly fail with this string unless you
convert it it back to UTF-16 and use wfstream instead.
Hi,

The user with the problem is in Russia with a Russian based keyboard.
Would that be the problem?

Thanks!------------------------
JeZxLee
JessePalser <AT> GMail <DOT> com
16BitSoft Inc.
Video Game Design Studio
www.16BitSoft.com

Ryan C. Gordon wrote:

Is it possible you have a user with a Unicode character (something with
an accent?) in their pref path? These will be converted to UTF-8 so they
fit in a const char * and work with strcpy() etc, but Windows
fstream::open() will almost certainly fail with this string unless you
convert it it back to UTF-16 and use wfstream instead.
Hi Again,

This is definitely the problem!
It’s a Russian based notebook where the save game data is not working.

Can someone explain how to convert from UTF-8 to UTF-16?
Here is the current code:

Code:
void Data::SaveHighScoresAndOptions(void)
{
char filename[1024];
fstream fileStream;
char textBuffer[50];
char *base_path = SDL_GetPrefPath(“16BitSoftInc”, “LettersFall-Retail”);
char *pref_path = NULL;

if (base_path)
{
    pref_path = SDL_strdup(base_path);
    SDL_free(base_path);
}
else  return;

strcpy_s(filename, pref_path);//settings->GetUserConfigPath() );
strcat_s(filename, "LettersFall-Alpha8-Data");

fileStream.open (filename, fstream::out);
if (fileStream.is_open())
{
	sprintf_s(textBuffer, "%d", audio->MusicVolume);
	fileStream<<textBuffer;
	fileStream<<"\n";

	sprintf_s(textBuffer, "%d", audio->SoundVolume);
	fileStream<<textBuffer;
	fileStream<<"\n";

Thanks!------------------------
JeZxLee
JessePalser <AT> GMail <DOT> com
16BitSoft Inc.
Video Game Design Studio
www.16BitSoft.com

Can someone explain how to convert from UTF-8 to UTF-16?
Here is the current code:
[snip]

You can convert from utf-8 to utf-16 with SDL_iconv_string, or
SDL_iconv_utf8_ucs2 should also work as a shorthand (though ucs2 is a
subset of utf16).

I think MSVC has a non-standard fstream constructor that takes a
wchar_t*, so you can try changing the line

fileStream.open(filename);

to

wchar_t* win_filename = (wchar_t*)SDL_iconv_utf8_ucs2(filename);
fileStream.open(win_filename);

and then add a SDL_free(win_filename) when you’re done with it.

This would be windows / MSVC specific though, so you might need to wrap
it in an ifdef or something.

You can also use the Windows function MultiByteToWideChar() with the
CP_UTF8 codepage flag.

https://msdn.microsoft.com/en-us/library/windows/desktop/dd319072(v=vs.85).aspx

Jonny DOn Wed, Apr 22, 2015 at 4:48 PM, Alex Baines wrote:

Can someone explain how to convert from UTF-8 to UTF-16?
Here is the current code:
[snip]

You can convert from utf-8 to utf-16 with SDL_iconv_string, or
SDL_iconv_utf8_ucs2 should also work as a shorthand (though ucs2 is a
subset of utf16).

I think MSVC has a non-standard fstream constructor that takes a
wchar_t*, so you can try changing the line

fileStream.open(filename);

to

wchar_t* win_filename = (wchar_t*)SDL_iconv_utf8_ucs2(filename);
fileStream.open(win_filename);

and then add a SDL_free(win_filename) when you’re done with it.

This would be windows / MSVC specific though, so you might need to wrap
it in an ifdef or something.


SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

The user with the problem is in Russia with a Russian based keyboard.
Would that be the problem?

Yeah, almost certainly using some character that’s not English A-Z. :slight_smile:

When you have the final string, right before you call fstream::open()
with this untested code:

#if WINDOWS
// (that should be “utf16” not “ucs2” but we don’t have that
// in SDL yet. We should add that for 2.0.4. ucs2 is good
// enough for now.)
wchar_t *wstr = (wchar_t *) SDL_iconv_utf8_ucs2(filename);
// make sure wstr isn’t NULL here.
wfstream fileStream;
fileStream.open(wstr, wfstream::out);
SDL_free(wstr);
#else
fstream fileSteam; // Linux and Mac OS X expect UTF-8 strings already.
fileStream.open(filename, fstream::out);
#endif

–ryan.

My understanding is that CP_UTF8 doesn’t work correctly on some versions
of Windows (SDL_iconv works everywhere, though).

–ryan.On 4/22/15 5:11 PM, Jonathan Dearborn wrote:

You can also use the Windows function MultiByteToWideChar() with the
CP_UTF8 codepage flag.

Hi,

I’ve got this now:

LOADING CODE:

Code:
void Data::LoadHighScoresAndOptions(void)
{
char filename[1024];
char textBuffer[50];
char *base_path = SDL_GetPrefPath(“16BitSoftInc”, “LettersFall-Retail”);
char *pref_path = NULL;

if (base_path)
{
    pref_path = SDL_strdup(base_path);
    SDL_free(base_path);
}
else  return;

strcpy_s(filename, pref_path);
strcat_s(filename, "LettersFall-Alpha9-Data");

#if __WINDOWS__
	wchar_t *wstr = (wchar_t *)SDL_iconv_utf8_ucs2(filename);
	wfstream fileStream;
	fileStream.open(wstr, wfstream::in);
	SDL_free(wstr);
#else
	fstream fileStream;
	fileStream.open(filename, fstream::in);
#endif 

if (fileStream.is_open())
{
	fileStream.getline (textBuffer, 30);
	audio->MusicVolume = (int)atoi(textBuffer);

	fileStream.getline (textBuffer, 30);
	audio->SoundVolume = (int)atoi(textBuffer);

SAVING CODE:

Code:
void Data::SaveHighScoresAndOptions(void)
{
char filename[1024];
char textBuffer[50];
char *base_path = SDL_GetPrefPath(“16BitSoftInc”, “LettersFall-Retail”);
char *pref_path = NULL;

if (base_path)
{
    pref_path = SDL_strdup(base_path);
    SDL_free(base_path);
}
else  return;

strcpy_s(filename, pref_path);
strcat_s(filename, "LettersFall-Alpha9-Data");

#if __WINDOWS__
	wchar_t *wstr = (wchar_t *)SDL_iconv_utf8_ucs2(filename);
	wfstream fileStream;
	fileStream.open(wstr, wfstream::out);
	SDL_free(wstr);
#else
	fstream fileStream;
	fileStream.open(filename, fstream::out);
#endif 

if (fileStream.is_open())
{
	sprintf_s(textBuffer, "%d", audio->MusicVolume);
	fileStream<<textBuffer;
	fileStream<<"\n";

	sprintf_s(textBuffer, "%d", audio->SoundVolume);
	fileStream<<textBuffer;
	fileStream<<"\n";

But it fails to compile?

Getting these error messages?:
Error 1 error C2664: ‘std::basic_istream<wchar_t,std::char_traits<wchar_t>> &std::basic_istream<wchar_t,std::char_traits<wchar_t>>::getline(_Elem *,std::streamsize,_Elem)’ :
cannot convert argument 1 from ‘char [50]’ to ‘wchar_t *’ C:\Users\Jesse Palser\Desktop\LFRetail-VS13\src\data.cpp 243 1 LFRetail-VS13

Almost there!------------------------
JeZxLee
JessePalser <AT> GMail <DOT> com
16BitSoft Inc.
Video Game Design Studio
www.16BitSoft.com

You only need to use a wfstream if you want to access the contents of
the file as wchar_t’s.

You probably want to continue using a fstream for both platforms, while
changing the windows code to pass the wchar_t filename to fstream::open.On 22/04/15 23:35, JeZ-l-Lee wrote:

Hi,

I’ve got this now:
[snip]
But it fails to compile?

Getting these error messages?:
Error 1 error C2664: ‘std::basic_istream<wchar_t,std::char_traits<wchar_t>> &std::basic_istream<wchar_t,std::char_traits<wchar_t>>::getline(_Elem *,std::streamsize,_Elem)’ :
cannot convert argument 1 from ‘char [50]’ to ‘wchar_t *’ C:\Users\Jesse Palser\Desktop\LFRetail-VS13\src\data.cpp 243 1 LFRetail-VS13

Alex Baines wrote:

You only need to use a wfstream if you want to access the contents of
the file as wchar_t’s.

You probably want to continue using a fstream for both platforms, while
changing the windows code to pass the wchar_t filename to fstream::open.
Hi,

Thanks!
We got it working now on the Russian notebook…------------------------
JeZxLee
JessePalser <AT> GMail <DOT> com
16BitSoft Inc.
Video Game Design Studio
www.16BitSoft.com

Hi,

We got this all fixed now, thanks to everyone who helped!

Here was the path on the Russian notebook with Win 8.1:

Code:
C:\Users\;5:A0=4@\AppData\Roaming\16BitSoftInc\LettersFall-Retail------------------------
JeZxLee
JessePalser <AT> GMail <DOT> com
16BitSoft Inc.
Video Game Design Studio
www.16BitSoft.com

Whoops, probably should have read MSDN more closely, sorry about that!

–ryan.On 04/22/2015 06:58 PM, Alex Baines wrote:

You only need to use a wfstream if you want to access the contents of
the file as wchar_t’s.