Re: [libgpiod][PATCH V9 0/8] libgpiod: Add Rust bindings

[Kent Gibson <warthog618@xxxxxxxxx>]

On Fri, Nov 18, 2022 at 04:44:56PM +0000, Gary Guo wrote:
> On Wed, 16 Nov 2022 09:12:01 +0800
> Kent Gibson <warthog618@xxxxxxxxx> wrote:
> 
> > On Mon, Nov 14, 2022 at 10:49:36PM +0100, Bartosz Golaszewski wrote:
> > > On Mon, Nov 7, 2022 at 10:57 AM Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:  
> > > >
> > > > Hello,
> > > >
> > > > Here is another version of the rust bindings, based of the master branch.
> > > >
> > > > Pushed here:
> > > >
> > > > https://github.com/vireshk/libgpiod v9
> > > >
> > > > V8->V9:
> > > > - Merged the last patch (supporting Events) with the other patches.
> > > > - Events implementation is simplified and made efficient. nth() is also
> > > >   implemented for the iterator.
> > > > - Unnecessary comment removed from Cargo.toml files.
> > > > - Updated categories in libgpiod's Cargo.toml.
> > > > - Updated gpio_events example to show cloned events live past another
> > > >   read_edge_events().
> > > > - Implement AsRawFd for Chip.
> > > > - Other minor changes.
> > > >  
> > > 
> > > Kent, Miguel: if you are ok with this version - can you add your review tags?
> > > 
> > > Bart  
> > 
> > As mentioned elsewhere, I'm a bit iffy about the handling of non-UTF-8
> > names, which are treated as errors, but are valid in the C API.
> > But that is an extreme corner case that can be addressed later, so I have
> > no objection to this version being merged.
> > 
> > Reviewed-by: Kent Gibson <warthog618@xxxxxxxxx>
> > 
> > Cheers,
> > Kent.
> 
> I have glanced through the code and I find no reason that the `str` used
> couldn't be replaced with `[u8]` or `OsStr`. The former might be a
> little bit difficult to use, but `OsStr` could be easily converted into
> `str` by just `.to_str().expect("name should be utf-8")` if the user
> don't care about non-UTF-8 or is certain that names are indeed UTF-8.
> 

Or you can use `OsStr::to_string_lossy()` though that requires allocation.

str is preferable as it is easer to use, and 99.99% of the time will be
fine.  I have toyed with using OsStr, but having to do the conversion
becomes really tedious really fast.

> I am not sure about whether this would be worthwhile though, because I
> doubt if anyone is actually using non-UTF-8 strings in those places.
> Non-ASCII usages should already be rare?
> 

That is what I meant by "an extreme corner case".

The problem with the GPIO uAPI is that it limits names to 32 bytes,
and valid UTF-8 passed in may get truncated mid-codepoint, resulting in
invalid UTF-8 when read back.
Handling that case by truncating the string before the split codepoint
would at least ensure that the Rust API would round-trip.

Cheers,
Kent.