TechnikTobi
commented
2 years ago Hi, thanks for your feedback!
Yes, now that you point it out I see the problem. Based on your input regarding symlink_file / symlink_inner and the two functions used for converting paths ( to_u16s and maybe_verbatim - which, as it turns out, also calls to_u16s here in a first attempt to convert the path into a vector of u16 values and optionally resolves any problematic characters. So, from what I can tell, maybe_verbatim should be more robust - why symlink_inner only uses to_u16s for one of its paths and not maybe_verbatim for both of them is therefore a mystery to me). Inside symlink_inner the converted paths are used to call the Win32 API function CreateSymbolicLinkW (Microsoft documentation) which takes the file names as LPCWSTR parameters - (long) pointers to constant wide strings (see here). At this point I don't that the inner workings of symlink_file are not of much help to solve this issue as gexiv2 works a bit differently:
Taking a look at what happens to the path after converting it to a CString (pointer) - for example, save_to_file calls gexiv2_metadata_save_file, which then calls convert_path see here to get a std::wstring (according to some C++ documentation this is a string using wchar_t for its characters, which seems to support UTF-16). This in turn calls g_utf8_to_utf16, a GLib function (documentation, source code) for converting UTF-8 strings to UTF-16. So, gexiv2 actually seems to support paths that use UTF-16 (at least to some extend), even though they need to be provided as UTF-8 strings. This raises some questions:
- Does this conversion provided by GLib support all UTF-16 characters (i.e. "all possible paths"), provided that they are first encoded into characters supported by UTF-8, e.g. by mapping a single UTF-16 character to a sequence of UTF-8 characters that can then be converted to the single UTF-16 character? Or is
g_utf8_to_utf16only able to map characters that are valid in both character sets? In that case fully supporting UTF-16 paths within rexiv2 does not really make sense if gexiv2 isn't able to handle them anyway. Of course, this does not mean that my previously suggested fix is still a good idea - I think that at this point some differentiation between windows and non-windows systems in the conversion might be a good idea, e.g. using cfg. - Assuming that
g_utf8_to_utf16is in fact able to convert "all possible paths": What would the corresponding r_utf16_to_utf8 on our side have to look like, i.e. how to encode the UTF-16 string as UTF-8 so thatg_utf8_to_utf16would be able to reconstruct the UTF-16 path? EDIT1: There was one thing I forgot to look up before writing this reply: Checking ifg_utf16_to_utf8already exists! This could potentially be useful. END EDIT1
Either way, to comment on your idea for a to_platform_path_ptr(): Of course something like that would be nice in general. However, even if such a function exists, I don't think it would be of much help in this specific case due to how gexiv2 further processes the provided path-strings.
Where to go from here? I think answering the questions above would be a helpful next step, so I'll try to look further into them - under the assumption that the questions themselves make sense and are not "ill-formed" due to some misunderstanding from my side (unfortunately however, with university starting again soon I'm afraid that progress will slow down on my side as well 😅).
Dalan94
felixc
To propose a solution for #59 I changed the creation of the c_str_path variables used in some functions (e.g.
new_from_path) by inserting a conversion ofOsStrtostrfirst, before callingas_bytes(), eliminating the need for the usage ofstd::os::unix::ffi::OsStrExt, making the library hopefully compile under windows (as I don't have a Windows machine at hand I could not test it yet, but as the errors mentioned in the issue only involve the usage of the unix related functions there is a good chance that this resolves the issue). I left a copy of the old version in as a comment (line 279) above the new conversion for comparison.Running all tests (using
cargo test) results in none of them failing. As at least one test includes a call tonew_from_path, where the new conversion gets used without problems, the proposed solution should be fine.