| Summary: | Windows implementation of GetBasePath is incorrect/unportable (wrong usage of GetModuleFileName) | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Coriiander <coriiander> |
| Component: | file | Assignee: | Ryan C. Gordon <icculus> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | minor | ||
| Priority: | P2 | ||
| Version: | 2.0.1 | ||
| Hardware: | All | ||
| OS: | Windows (All) | ||
| Attachments: | Proposed fix. | ||
|
Description
Coriiander
2014-03-07 09:47:55 UTC
I forgot to add the following: Also keep in mind that GetModuleFileName(Ex) has some varying issues between 32/64-bit/WOW64. Also its result is different with regards to the null-terminator character on Windows XP. Also the output is not necessarily a full path, but could be a short-filename, or subject to path aliassing. I expect the goal of such function is to provide the user with a useable, reliable full path, therefor I suggest to ensure to return a useable, reliable, full absolute path, in such a way that it is portable between Windows-versions. Unfortunately this is quite some work for a little, though much welcome feature, but as it is now, you should just return NULL or remove it from the public API. Hey Ryan, how critical is this? (In reply to Sam Lantinga from comment #2) > Hey Ryan, how critical is this? Worth fixing for 2.0.3, but not a showstopper. --ryan. Marking a large number of bugs with the "triage-2.0.4" keyword at once. Sorry if you got a lot of email from this. This is to help me sort through some bugs in regards to a 2.0.4 release. We may or may not fix this bug for 2.0.4, though! (sorry if you get a lot of copies of this email, I'm marking several bugs at once) Marking bugs for the (mostly) final 2.0.4 TODO list. This means we're hoping to resolve this bug before 2.0.4 ships if possible. In a perfect world, the open bug count with the target-2.0.4 keyword is zero when we ship. (Note that closing a bug report as WONTFIX, INVALID or WORKSFORME might still happen.) --ryan. Created attachment 2164 [details]
Proposed fix.
Here's a patch that _should_ fix these concerns. I'm posting this to Bugzilla because I don't have a Windows machine handy to test it right now.
--ryan.
Fixed, and committed: https://hg.libsdl.org/SDL/rev/0e1f57b051f4 https://hg.libsdl.org/SDL/rev/bc1ba207ff16 |