search

smoltcp-rs / smoltcp

a smol tcp/ip stack
BSD Zero Clause License
3.63k stars 402 forks source link

Ergonomics of `Interface::new` device parameter #929

Open TomCrypto opened 1 month ago

TomCrypto commented 1 month ago

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?

chenzhuoyu commented 1 month ago

@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.

thvdveld commented 1 month ago

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.

Dirbaio commented 1 month ago

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.

whitequark commented 1 month ago

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.