Re: [libgpiod][PATCH v2 1/3] bindings: rust: fix soundness of line_info modeling

["Erik Schilling" <erik.schilling@xxxxxxxxxx>]

On Tue Oct 3, 2023 at 10:58 AM CEST, Viresh Kumar wrote:
> On 29-09-23, 15:18, Erik Schilling wrote:
> > diff --git a/bindings/rust/libgpiod/src/chip.rs b/bindings/rust/libgpiod/src/chip.rs
> > index 81e1be6..9ef8f22 100644
> > --- a/bindings/rust/libgpiod/src/chip.rs
> > +++ b/bindings/rust/libgpiod/src/chip.rs
> > @@ -107,7 +107,9 @@ impl Chip {
> >              ));
> >          }
> >  
> > -        line::Info::new(info)
> > +        // SAFETY: We verified that the pointer is valid. We own the pointer and
> > +        // no longer use it after converting it into a Info instance.
> > +        Ok(unsafe { line::Info::from_raw_owned(info) })
>
> Hmm, I was expecting the naming to be simplified in this version here.
>
> Now:
>
> Info::from_raw_owned()
> InfoRef::from_raw_non_owning()
>
> What I am suggesting:
>
> Info::from_raw()
> InfoRef::from_raw()
>
> Or maybe just `new()` routines for both ?
>
> I think structure names tell us enough about ownership here and we don't need to
> add it to functions.

Ah, I posted some weak opposition against that in [1], but failed to
mention that again for v2. I mostly disliked `Info::from_raw()` being
not very expressive without peeking at the type definition. But if you
feel strongly about this, I am happy to change it.

I would strongly vote for `from_raw()` since `new()` sounds like it
would create a new instance, but really, we are just wrapping an already
existing instance here.

So... Shall I move to `from_raw()` or keep
`from_raw_owned/non_owning()`?

- Erik

[1] https://lore.kernel.org/r/CVUJTBQZYN6B.17WXH28G8MKZ2@ablu-work