Changing Original Library API when writing Wrappers

[Jos van den Oever]

A small explanation for people that wonder why converting from the Rust 
type std::path::Path to std::string::String can fail. 
std::string::String is always UTF-8, but std::path::Path is not because
not all filesystems encode paths in UTF-8 [0].

In Qt, QString is (possibly invalid) utf16. Opening files is done with 
QString. This code give no compile-time and no runtime error:
     char filename[] = { 0x01, 0x02, 0x03, 0x5c };
     QString::fromLocal8Bit(filename);

So converting from std::path::Path to QString does not need to lead to 
an error. QString can take any bytearray.

This does not change the main point: in Rust, functions can return 
errors. For `write_path_entry`, the signature still needs an error. That 
can be either a std::io::Error or a custum Error struct for this binding 
crate.

Cheers,
Jos

[0] On Unix a filename is any bytearray without 0x00 and 0x2f. Try this:

==8<== Write a file with non-utf8 path
#include <stdio.h>

int
main() {
     char filename[] = { 0x01, 0x02, 0x03, 0x5c };
     FILE *fh = fopen(filename, "w");
     if (!fh) {
         printf("Could not create file '%s'", filename);
         return 1;
     }
     fclose(fh);
     return 0;
}
==8<== Read the file again with Qt code
#include <QtCore/QFile>
#include <QtCore/QString>

int
main() {
     char filename[] = { 0x01, 0x02, 0x03, 0x5c };
     QString qfilename = QString::fromLocal8Bit(filename);
     QFile file = { qfilename };
     if (file.open(QIODevice::ReadOnly)) {
         printf("Could open the file.");
     } else {
         printf("Could not open the file.");
     }
     return 0;
}
==8<== Read the file again with Rust code
use std::os::unix::ffi::OsStringExt;

fn main() {
     let filename = vec![1,2,3,92];
     let os_path = std::ffi::OsString::from_vec(filename);
     let path = std::path::PathBuf::from(os_path);
     if let Ok(_contents) = std::fs::read(&path) {
         println!("Read '{}'.", path.display());
     } else {
         println!("Could not read '{}'.", path.display());
     }
}
===

Ayush Singh schreef op 2022-01-31 04:51:

> Ok, so for a more involved example, take the case of a method in 
> KConfigGroup:
> `void writePathEntry (const QString &pKey, const QString &path,
> WriteConfigFlags pFlags=Normal)`
> 
> Here, the path has a type of QString. In Rust, we can instead use the
> type `Path` to represent this and convert it to `QString` using
> `TryFrom` trait before passing to C++.
> 
> This change would mean that first a `PathBuf` or `Path` is constructed
> and then is converted to `QString`. Since this conversion can fail, we
> will need to return a `Result`. The function definition for this new
> function will be something like this:
> ```
> pub fn write_path_entry(&mut self, pkey: QString, path: &Path, pflags:
> WriteConfigFlags) -> Result<(), Error> {
> let path = QString::try_from(path)?;
> .....
> Ok(())
> }
> ```
> 
> Here, the API changes significantly. If the path is being allocated at
> runtime, it incurs the cost of constructing `PathBuf` on the heap
> along with the normal cost of `QString`. However, this path is now
> significantly more useful if we have to interact with Rust libraries.
> Also, it automatically checks that the path actually looks like a Path
> instead of just any random String. Since we are passing a reference,
> we do not have to clone the `Path`, unlike `QString`, so I don't think
> the cost should be much more than just QString for multiple uses.
> 
> So what do you suggest in this case? I think most medium to big
> projects would use `Path` regardless but small projects might end up
> using QString. It is also an option to keep both forms with different
> function names.
> 
> Ayush Singh
> 
> On Mon, Jan 31, 2022 at 4:13 AM David Hurka <david.hurka at mailbox.org> 
> wrote:
> On Sunday, January 30, 2022 2:20:10 PM CET Ayush Singh wrote: An 
> example of this situation:
> `QString readGenericName () const`
> method of KDesktopFile
> (https://api.kde.org/frameworks/kconfig/html/classKDesktopFile.html#aaf263b7
> 9cce3125c9e6e52428e05c524). If I am doing a one-to-one wrapping, the 
> Rust
> function definition
> would look something like this:
> `fn read_generic_name(&self) -> QString`
> By default, this function returns `QString()` in case the key is not
> present in the `.desktop` file. According to me, a better definition
> for this function will be:
> `fn read_generic_name(&self) -> Option<QString>`
> The function will return `None` if the key is not present in this case.
> I remember that there were plans to use std::optional<> for return 
> types in a
> future version, so KConfig would end up to use the C++ interface you 
> propose
> for Rust. So your suggestion seems appropriate in this special case.
> 
> Besides that, I think that this example is fairly low-level. If you 
> adapt
> towards Rust here, you preserve the overall way to use the library, and 
> you
> just have a more elegant way to reach your goal.
> 
> Cheers, David