Skip to content
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

extract_path_parts when no directory is specified #26

Open
dancrepeau opened this issue Feb 18, 2021 · 2 comments
Open

extract_path_parts when no directory is specified #26

dancrepeau opened this issue Feb 18, 2021 · 2 comments

Comments

@dancrepeau
Copy link
Contributor

I found that extract_path_parts on Windows (_Win32) is broken in certain cases. If no path is specified, then the code fails when looking for the extension, because it iterates until it finds a backlash (which it never does). This causes unpredictable results.

I think the solution is to build in path generate code in cases where an absolute path is not specified. This is done in the unix version with the line (cwd = getenv("PWD")) which is then copied into the beginning of the path. This is done (in unix) in cases where the path does not begin with a forward slash.

Doing something similar in Windows might be tricky, since the path could start with C:\ or c:\ or D:\ or d:, or whatever. But even if we don't put in a full path, we need to fix the case where it can't parse the extension (because there is no extension).

I'd be happy to tackle this problem, but wanted to mention it in case anyone had strong opinions about it.

@cimbi
Copy link
Member

cimbi commented Feb 19, 2021

Hi Dan,
feel free to edit that

@dancrepeau
Copy link
Contributor Author

Great. What I'm looking to do is, replace this:

slash_to_backslash(full_file_name);
MEF_strncpy(temp_full_file_name, full_file_name, MEF_FULL_FILE_NAME_BYTES); // do non-destructively

with this:

MEF_strncpy(temp_full_file_name_backslash, full_file_name, MEF_FULL_FILE_NAME_BYTES); // do non-destructively
slash_to_backslash(temp_full_file_name_backslash);
GetFullPathNameA(temp_full_file_name_backslash, MEF_FULL_FILE_NAME_BYTES, temp_full_file_name, lppPart);

This seems to work (turning a relative path into a full path), and also truly makes it non-destructive. Before, the comment ("do non-destructively") was wrong because we had already removed the backslashes in the previous line of code. The documentation for the function in the spec says the input shouldn't be modified, so unless this breaks something that's relying on the backslashes later on, I'd be inclined to fix it in this manner.

This is the non-unicode version of the function, which is similar to what we did with the generate_file_list() changes a while back. Supporting the unicode version requires different character type definitions. Another option would be to not use the Microsoft GetFullPathName at all, but rather just check for "\" or "c:" at the beginning of a path ('c' or any other letter drive).

-Dan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants