API issue with UnknownOption

[leshow]

We have a bit of an API issue with the UnknownOption

The fact that this doesn't work, is perhaps a little unexpected:

        let mut opts = DhcpOptions::new();
        opts.insert(DhcpOption::Unknown(UnknownOption {
            code: 1,
            data: vec![192, 168, 1, 1],
        }));
        let opt = opts.get(OptionCode::SubnetMask);
        dbg!(opt); // None

We can modify From<&DhcpOption> for OptionCode' so that the key in the map is a OptionCode::SubnetMask by changing the unknown branch from:

Unknown(n) => OptionCode::Unknown(n.code),

to

Unknown(n) => n.code.into(),

This way you don't need to retrieve the option value with OptionCode::Unknown(1) which feels pretty weird. But the value you will get back after making the above change for the original code will be:

[src/v4/options.rs:1950] opt = Some(
    Unknown(
        UnknownOption {
            code: 1,
            data: [
                192,
                168,
                1,
                1,
            ],
        },
    ),
)

Again, this feels a little strange. The Unknown option could have been inserted programmatically and unbeknownst to the programmer, if let Some(DhcpOption::SubnetMask(_)) = opts.get(OptionCode::SubnetMask) will not work.

We could decode UnknownOption on insert but that feels pretty icky and would likely require re-allocated to prefix the code u8 at the beginning of the data section.

If anyone has some better ideas, feel free to share

edit:

Another option, we could use the repr(u8) for OptionCode and store the keys as the u8 value. That would solve half of this problem. We may want to do this as part of more general option generating macros described in #43

[Jackbaude]

I can maybe take a look at this!

[leshow]

Sure! I have not really come to any conclusions about what to do, but I'm open to suggestions. If you have ideas, maybe suggest what that could look like here before you sink time into it. We could also merge the repr change as a first step and go from there, at least then .get(Unknown(1)) would work regardless of how the option was inserted.

Another thought:

It might be interesting to not actually parse the option data section at the time of message decode. Instead, we could concat any long form encoding opts but keep everything as UnknownOption, then on get we could parse the result? Or maybe, get always returns Option<&UnknownOption> but there is a way to decode/convert into its specific variant, or just a method on UnknownOption that returns its parsed data.

We could provide this as a GenericMap along with the one we have today, where the generic version doesn't decode each options data section as above.