smoltcp icon indicating copy to clipboard operation
smoltcp copied to clipboard

Ergonomics of `Interface::new` device parameter

Open TomCrypto opened this issue 1 year ago • 4 comments
trafficstars

Currently the Interface struct needs a Device to construct itself via Interface::new, but all it does with it is call capabilities on it. In some cases (e.g. embedded code) we would like to instantiate the Interface before the device object can be constructed, even though we typically know the device capabilities upfront.

Should Interface::new directly take a DeviceCapabilities parameter instead of a Device?

TomCrypto avatar May 13 '24 12:05 TomCrypto

@TomCrypto I worked around this problem by creating a "dummy" device with the only purpose of providing the DeviceCapabilities, and it works fine. But I am certainly wish there be a better solution.

chenzhuoyu avatar May 22 '24 12:05 chenzhuoyu

We indeed don't use the device when creating the interface, only the capabilities. We could indeed modify the function to take the capabilities instead of the device. However, I'm not sure about the implications. Maybe the reason why it was taking in a device was to have ownership of it. But I'm not sure if that is really necessary. Pinging @whitequark and @Dirbaio for this.

thvdveld avatar May 23 '24 06:05 thvdveld

Originally Interface would permanently own the Device. Later I changed it to taking &mut Device in method calls. It's true creation only needs the DeviceCapabilities, but I thought taking the DeviceCapabilities directly could be footgunny, so I kept the entire Device. A footgun example is if new takes DeviceCapabilities it "looks like" you can specify/set the capabilities yourself to anything you want, while in reality you're obligated to pass the device's, and you'll get weird incorrect behavior if you don't.

I'm not opposed to the change if others think it's a good idea, it just makes me a bit uncomfortable.

Dirbaio avatar May 24 '24 13:05 Dirbaio

I'm not opposed to the change if others think it's a good idea, it just makes me a bit uncomfortable.

I agree. I think originally I had it own the Device because then correctness is guaranteed by construction. But also, isn't it true that if you pass a &mut Device it could be a different one each time?

One possibility is adding a debug_assert!() that the device capabilities match each time you poll.

whitequark avatar May 24 '24 22:05 whitequark