New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Joystick fixes/update for non-MSVC compilers #706

Merged
merged 1 commit into from Oct 2, 2014

Conversation

Projects
None yet
3 participants
@MarioLiebisch
Member

MarioLiebisch commented Sep 25, 2014

This PR is in addition to #701 and #702:

  • This fixes building on non-MSVC compilers for Windows since they don't know _tcsnlen().
  • Changed logic so SFML tries to retrieve the paths from the user key first, machine key second (typical behavior for most registry settings).
  • Removed the registry key length checks since that check is performed insideRegOpenKeyEx() anyway and cut-off keys might point to the wrong keys.
  • Updated the error string retrieval to properly handle errors and not include a trailing line break.
Joystick fixes/update for non-MSVC compilers
* This fixes building on non-MSVC compilers for Windows since they don't
know `_tcsnlen()`.
* Changed logic so SFML tries to retrieve the paths from the user key
first, machine key second (typical behavior for most registry settings).
* Removed the registry key length checks since that check is performed
inside`RegOpenKeyEx()` anyway and cut-off keys might point to the wrong
keys.
* Updated the error string retrieval to properly handle errors.
@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Sep 25, 2014

Member

I tested my refactor on MinGW first, and _tcsnlen is defined in the supplied tchar.h. If you are talking about cygwin environments, they emulate a UNIX environment on Windows, so trying to access the registry through cygwin is probably not such a good idea anyway. But yeah... I guess there was no point checking the length of the string anyway since the function would only return values smaller than 255 if there was already a null somewhere.

since that check is performed inside RegOpenKeyEx() anyway

Is this officially documented somewhere?

Also, initially I figured relying on manual win32 memory management with FormatString() was a bad idea, but I guess it sucks more if an error message ends up being truncated.

And lastly, are all those TEXT('\\') really necessary? 😛 I mean... in the end TEXT("\\") gets expanded to L"" anyway, and L'' doesn't look as nice to me 😉.

Member

binary1248 commented Sep 25, 2014

I tested my refactor on MinGW first, and _tcsnlen is defined in the supplied tchar.h. If you are talking about cygwin environments, they emulate a UNIX environment on Windows, so trying to access the registry through cygwin is probably not such a good idea anyway. But yeah... I guess there was no point checking the length of the string anyway since the function would only return values smaller than 255 if there was already a null somewhere.

since that check is performed inside RegOpenKeyEx() anyway

Is this officially documented somewhere?

Also, initially I figured relying on manual win32 memory management with FormatString() was a bad idea, but I guess it sucks more if an error message ends up being truncated.

And lastly, are all those TEXT('\\') really necessary? 😛 I mean... in the end TEXT("\\") gets expanded to L"" anyway, and L'' doesn't look as nice to me 😉.

@MarioLiebisch

This comment has been minimized.

Show comment
Hide comment
@MarioLiebisch

MarioLiebisch Sep 25, 2014

Member

I tested my refactor on MinGW first, and _tcsnlen is defined in the supplied tchar.h.

Mine didn't. I'm using the 32 bit version of 4.8.1 provided by mingw.org using their mingw-get tool.

Is this officially documented somewhere?

There's an error returned in case the passed path is invalid in some way. Possible error values (and meanings) aren't documented as far as I know, MSDN just tells you to ask FormatMessage() for the actual error string.

And lastly, are all those TEXT('') really necessary?

I removed them all before and called the wide char version of the functions, but then I noticed that regstr.h also uses those macros so I thought it might be better to keep it consistent here (also in case someone forces the non-unicode version). I've changed those to single character values to just avoid concatenating/parsing them as strings.

Member

MarioLiebisch commented Sep 25, 2014

I tested my refactor on MinGW first, and _tcsnlen is defined in the supplied tchar.h.

Mine didn't. I'm using the 32 bit version of 4.8.1 provided by mingw.org using their mingw-get tool.

Is this officially documented somewhere?

There's an error returned in case the passed path is invalid in some way. Possible error values (and meanings) aren't documented as far as I know, MSDN just tells you to ask FormatMessage() for the actual error string.

And lastly, are all those TEXT('') really necessary?

I removed them all before and called the wide char version of the functions, but then I noticed that regstr.h also uses those macros so I thought it might be better to keep it consistent here (also in case someone forces the non-unicode version). I've changed those to single character values to just avoid concatenating/parsing them as strings.

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Sep 25, 2014

Member

Yeah... I'm using 4.9.0 from mingw-builds... I guess they might have added it in themselves then.

Hmm... I was under the impression that RegOpenKeyEx() would just silently fail or even worse crash if it was fed over-long keys. If it plays nice and returns an error, I can live with that 😄.

also in case someone forces the non-unicode version

They would end up breaking more things than this 😉. But, yeah... I guess if you wanted to be really picky, the single character appending should run just a tiny bit faster than the string appends 😛, that is if the single character TEXT() aren't expanded to strings anyway somewhere.

Member

binary1248 commented Sep 25, 2014

Yeah... I'm using 4.9.0 from mingw-builds... I guess they might have added it in themselves then.

Hmm... I was under the impression that RegOpenKeyEx() would just silently fail or even worse crash if it was fed over-long keys. If it plays nice and returns an error, I can live with that 😄.

also in case someone forces the non-unicode version

They would end up breaking more things than this 😉. But, yeah... I guess if you wanted to be really picky, the single character appending should run just a tiny bit faster than the string appends 😛, that is if the single character TEXT() aren't expanded to strings anyway somewhere.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Oct 2, 2014

Member

This PR has been added to my merge list. Please test this and/or comment if something needs adjusting.

Edit: Fixes the MinGW issue and works fine.

Member

eXpl0it3r commented Oct 2, 2014

This PR has been added to my merge list. Please test this and/or comment if something needs adjusting.

Edit: Fixes the MinGW issue and works fine.

@eXpl0it3r eXpl0it3r merged commit c36ea07 into SFML:master Oct 2, 2014

@eXpl0it3r eXpl0it3r added s:accepted and removed s:undecided labels Oct 2, 2014

@eXpl0it3r eXpl0it3r added this to the 2.2 milestone Oct 2, 2014

@eXpl0it3r eXpl0it3r self-assigned this Oct 2, 2014

@MarioLiebisch MarioLiebisch deleted the MarioLiebisch:joyfix branch Oct 3, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment