Skip to content

WIP: network: New Serialize, Deserialize, and GetSerializedSize for Ipv[4,6]Address

As discussed in the ns-dev mailing list, Ipv[4,6]Address::Serialize and Ipv[4,6]Address:Deserialize violate the static polymorphism. In other terms, they have the same name as Header::Serialize and Header::Deserialize but different arguments.

This patch deprecates them and adds 5 now functions:

  • uint32_t CopyTo (uint8_t *buffer) const;
  • uint32_t CopyFrom (const uint8_t *buffer, uint8_t len);
  • uint32_t GetSerializedSize (void) const;
  • void Serialize (Buffer::Iterator &start) const;
  • uint32_t Deserialize (Buffer::Iterator &start);

The problem is that I'm still unhappy with the changes, as even the new Serialize and Deserialize are not perfectly in-line with the ones in Header. These use a copy of the buffer iterator. In the Address ones we use a reference (to avoid to manually have to increase the iterator after each call).

Note that using a reference in Serialized and Deserialized is used also in other places, and I don't like it either.

Said so, and given that everything that can be done with Serialize and Deserialize can be done using CopyTo and CopyFrom - albeit in a little less elegant way - my proposal would be to kill GetSerializedSize, Serialize, and Deserialize altogether, and leave only CopyFrom and CopyTo (which, btw, are defined in Address, and every Address-like object should have).

Moreover, just to simplify some code, I'd add a new constructor to Ipv4Address:

  • Ipv4Address (uint8_t address[4]); because... well, because it's what the actual Ipv4Address::Deserialize() does (remember, it's a static function): construct a new object with a given array of bytes.

Opinions ?

I'd like to hear especially the ones of @natale-p and @pdbj,since they did add useful comments to the mailing list.

Merge request reports