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 actualIpv4Address::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.