Pull requests

#6 Declined
Repository
Deleted repository
Branch
default (cfd60f60815c)
Repository
VoiDeD/Open Steamworks Open Steamworks
Branch
default

fix GetSteamID proto type error in ISteamUser014.

Author
  1. Lei Shi
Reviewers
Description

Modify prototype of GetSteamID. previous prototype wouldn't work correctly if compile with Visual Studio 2010, Because the SteamID is returned by register EAX and EDX instead of a auto generated hidden parameter which point to a SteamID.

  • Learn about pull requests

Comments (3)

  1. Didrole

    Your modifications won't even compile because you didn't mark your functions as virtual.

    Also I haven't noticed any problem with the current prototype and VS2010 (I've just compiled and run a test project successfully).

    Can you provide more details (and a sample code) ?

  2. Lei Shi author

    Sorry, I've fixed that. Forgive my miss testing. I didn't try to build whole Open Steamworks, instead, I just utilize the interface part with a minor modification. Maybe the problem is due to that I typedef SteamID as uint64.

    So the code below is legal in my project:

    SteamID GetSteamID()
    {
        return 0;
    }
    

    Checking the generated code, you will see code like this:

    ...
    xor eax, eax
    xor edx, edx
    retn
    

    Actually, the correct and expected code should pass the value out by a hidden parameter which is a pointer to a SteamID instance owned by caller. MSVC will generated such parameter if return value is a class(structure). so I think my modified prototype is a little bit safer. because it doesn't reply on compiler Intrinsics.

  3. Asher Baker

    The original is valid and conforms to the ABI, this isn't ill-defined behaviour. Additionally, OSW is designed for calling into SteamWorks, not reimplementing, I'm fairly sure your changes will upset the compiler for our target use - not to mention GCC as well. It would probably require some horrible compiler-specific calling convention hackery to have it work with both non-POD and POD types, although I'm fairly surprised MSVC managed to optimize it to that.