Newbie :: explain different behavior

help
1

I am using smoltcp-rs smoltcp · GitHub

I have created 4 functions doing similar stuff, however parse_udp enforced me to use the lifetime while previous calls didn't.
But finally parse_dhcp isn't working because "returns a value referencing data owned by the current function"

Please explain me the differences...

fn parse_ethernet(payload: &[u8])
            -> Result<(&[u8], EthernetRepr), PacketParseError> {
    let eth_frame = 
        EthernetFrame::new_checked(payload)
            .map_err(PacketParseError::EthernetError)?;
    let eth_repr = 
        EthernetRepr::parse(&eth_frame)
            .map_err(PacketParseError::EthernetError)?;
    Ok((eth_frame.payload(), eth_repr))
}

fn parse_ipv4(payload: &[u8])
              -> Result<(&[u8], Ipv4Repr), PacketParseError> {
    let ipv4_packet =
        Ipv4Packet::new_checked(payload)
            .map_err(|e| PacketParseError::Ipv4PacketError(e))?;

    let ipv4_repr =
        Ipv4Repr::parse(&ipv4_packet, &ChecksumCapabilities::ignored())
            .map_err(PacketParseError::Ipv4ReprParseError)?; // shorthand for |e| PacketParseError::Ipv4ReprParseError(e)
    Ok((ipv4_packet.payload(), ipv4_repr))
}

fn parse_udp<'a>(payload: &'a [u8], ipv4_repr: &Ipv4Repr)
                 -> Result<(&'a [u8], UdpRepr), PacketParseError> {
    let udp_packet =
        UdpPacket::new_checked(payload)
            .map_err(PacketParseError::UdpPacketError)?;
    let udp_repr =
        UdpRepr::parse(
            &udp_packet,
            &IpAddress::from(ipv4_repr.src_addr), // Convert Ipv4Address to IpAddress
            &IpAddress::from(ipv4_repr.dst_addr), // Convert Ipv4Address to IpAddress
            &ChecksumCapabilities::ignored())
        .map_err(PacketParseError::UdpReprParseError)?;
    Ok((udp_packet.payload(), udp_repr))
}

fn parse_dhcp(payload: &[u8])
              -> Result<DhcpRepr, PacketParseError> {
    let dhcp_packet =
        DhcpPacket::new_checked(payload)
            .map_err(PacketParseError::DhcpPacketError)?;
    let dhcp_repr =
        DhcpRepr::parse(&dhcp_packet)
            .map_err(PacketParseError::DhcpReprParseError)?;
    Ok(dhcp_repr) // this gives an error :: returns a value referencing data owned by the current function
}

Thanx.

2

Please post the signature of DhcpRepr::parse. If the lifetime of the input param reference is also the lifetime of the return value, that would explain the error.

Also, be sure to always post full errors -- the error from running cargo check or cargo build in the terminal, if your IDE is not giving you the full error. It often has details and hints that help in diagnosing the problem.

3

The DhcpRepr borrows the DhcpPacket (not just the data it borrows). Therefore, you cannot return a DhcpRepr from a function that doesn't take a DhcpPacket as input.

2 Likes
4

Then why would it work for Ipv4Repr but not DhcpRepr?

5

Ipv4Repr does not contain any references, and does not borrow from any of its input parameters. Note that it has no lifetime annotation.

1 Like
6

I see and that will be the issue, that the developer of that lib implemented the same thing differently because of "sometimes" introducing a lifetime.

Question remains: how can I return Ipv4Repr in that method?

7

I'm confused since that worked. Did you mean DhcpRepr?

8

indeed...
Question remains: how can I return DhcpRepr in that method?

9

You'll have to split it into two parts (not compiled):

fn parse_dhcp_packet(payload: &[u8])
              -> Result<DhcpPacket, PacketParseError> {
        DhcpPacket::new_checked(payload)
            .map_err(PacketParseError::DhcpPacketError)
}

fn parse_dhcp_repr(packet: &DhcpPacket)
              -> Result<DhcpRepr, PacketParseError> {
        DhcpRepr::parse(packet)
            .map_err(PacketParseError::DhcpReprParseError)
}

let packet = parse_dhcp_packet(payload)?;

let repr = parse_dhcp_repr(&packet)?;

This (perhaps inconveniently) means you have to keep around the packet variable for as long as you're accessing the repr variable since the latter borrows from the former. And you can't store them in the same struct since that would be a self-referential struct, which is not supported in Rust.

2 Likes
10

As all the fields are public, there's an alternative[1] if you're willing to discard the borrowed fields:

fn parse_dhcp(payload: &[u8]) -> Result<DhcpRepr, PacketParseError> {
    let dhcp_packet =
        DhcpPacket::new_checked(payload)
            .map_err(PacketParseError::DhcpPacketError)?;

    let dhcp_repr =
        DhcpRepr::parse(&dhcp_packet)
            .map_err(PacketParseError::DhcpReprParseError)?;

    // Discards `parameter_request_list` and `additional_options`
    Ok(DhcpRepr {
        parameter_request_list: None,
        additional_options: &[],
        ..dhcp_repr
    })
}

You could convert those fields to something owned (e.g. Option<Vec<u8>> for the parameter_request_list) and return that as well, if you need the data. You'd have to reconstruct the DhcpRepr fields elsewhere probably.

Edit: You can actually return DhcpRepr<'static> with this approach, since you're not borrowing anything in DhcpRepr anymore (but then it may be a breaking change to go back to being borrowing if upstream improves in the way discussed below).

Better solutions are only possible by changing upstream. I believe they could improve things in a non-breaking manner by adding some method like:

impl<'a, T: AsRef<[u8]> + ?Sized> DhcpPacket<&'a T> {
    pub fn options_redone(&self) -> impl Iterator<Item = DhcpOption<'a>> + use<'a, T> {
        // ...
    }
}

and then based on the current implementation they could

impl<'a> DhcpRepr<'a> {
-    pub fn parse<T>(packet: &'a Packet<&'a T>) -> Result<Self>
+    pub fn parse<T>(packet: &Packet<&'a T>) -> Result<Self>
     where
         T: AsRef<[u8]> + ?Sized,
     {
        ...

-        for option in packet.options() {
+        for option in packet.options_redone() {

which would let your OP compile.

  1. disclaimer: I only tested a mock up ↩︎

1 Like