Bug 634

Created attachment 525 [details]
Updated, forgot the const.

(In reply to comment #0)
> Created an attachment (id=524) [details]
> Adds GetMaskLength-method.
> 
> For some stuff, I need to check what size a given network is, "prefix length",
> ie. the CIDR notation, or 1.2.3.0/24 and similar. This patch adds a
> GetMaskLength() method to Ipv4Mask that obtains it. Documentation included. 
> 
> This notation is also used in IPv6 prefixes. (Weirdly, there is no Ipv4Prefix
> class).
> 
> For example,
> 
> std::cout << Ipv4Mask ("255.255.255.255").GetMaskLength() 
> 
> prints 32,
> 
> std::cout << Ipv4Mask ("255.255.255.240").GetMaskLength() 
> 
> prints 28
> 
> std::cout << Ipv4Mask ("128.0.0.0").GetMaskLength() 
> 
> prints 1
> 
> std::cout << Ipv4Mask ("0.0.0.0").GetMaskLength() 
> 
> prints 0.
> 
> Anyway, just in case it seems a good addition.
> 
> Unfortunately, the constructor for Ipv4Mask that takes an integer as a
> parameter, thus it cannot easily be adapted into specifying length instead,
> it's simply copying a 32-bit number as the mask. I would like to enter
> Ipv4masks in prefix format.

What about improving the implementation to detect a leading slash, and also allow these strings?

Ipv4Mask ("/32")
Ipv4Mask ("/24")
etc.
> 
> (Ipv6Prefix::Ipv6Prefix(uint8_t) DOES exist). 
> 
> Also Ipv6Prefix class didn't seem to have a convention for function name to
> simply get the length, so I used this one.
> 
I didn't quite understand the above comment-- you are saying that you invented "GetMaskLength()" because there was no corresponding "GetPrefixLength()" function in IPv6?  Should you also add a similar length function to Ipv6Prefix?


    /**
+    * \brief Get Prefix length of mask (the yy in x.x.x.x/yy notation)
+    *
+    * Prints mask in Prefix Length notation
+    */
+    
+   uint16_t GetMaskLength (void) const;

For this doxygen, I would use \return instead of \brief and delete the "Prints mask in Prefix Length notation" statement.

(In reply to comment #2)
> What about improving the implementation to detect a leading slash, and also
> allow these strings?
> 
> Ipv4Mask ("/32")
> Ipv4Mask ("/24")
> etc.

  Sounds like a good idea. 

> > (Ipv6Prefix::Ipv6Prefix(uint8_t) DOES exist). 
> > 
> > Also Ipv6Prefix class didn't seem to have a convention for function name to
> > simply get the length, so I used this one.
> I didn't quite understand the above comment-- you are saying that you invented
> "GetMaskLength()" because there was no corresponding "GetPrefixLength()"
> function in IPv6?  Should you also add a similar length function to
> Ipv6Prefix?

  Since Ipv4 stuff seems to talk about Masks (class name: Ipv4Mask) and Ipv6 talks about prefixes (Ipv6Prefix) I went with "Mask" here just to keep with what seemed to be convention on Ipv4 side. So it's just whether this should be called GetPrefixLength() or GetMaskLength().

  And yeah, I would eventually need this on Ipv6 side as well, should be easy enough to implement as well. 

(In reply to comment #3)
> (In reply to comment #2)
> > What about improving the implementation to detect a leading slash, and also
> > allow these strings?
> > 
> > Ipv4Mask ("/32")
> > Ipv4Mask ("/24")
> > etc.
> 
>   Sounds like a good idea. 
> 
> > > (Ipv6Prefix::Ipv6Prefix(uint8_t) DOES exist). 
> > > 
> > > Also Ipv6Prefix class didn't seem to have a convention for function name to
> > > simply get the length, so I used this one.
> > I didn't quite understand the above comment-- you are saying that you invented
> > "GetMaskLength()" because there was no corresponding "GetPrefixLength()"
> > function in IPv6?  Should you also add a similar length function to
> > Ipv6Prefix?
> 
>   Since Ipv4 stuff seems to talk about Masks (class name: Ipv4Mask) and Ipv6
> talks about prefixes (Ipv6Prefix) I went with "Mask" here just to keep with
> what seemed to be convention on Ipv4 side. So it's just whether this should be
> called GetPrefixLength() or GetMaskLength().

I elected to change this to GetPrefixLength(), for future IPv6 alignment, and because prefix length is a common term in IPv4 (strictly speaking, mask length is 32 bits, I suppose).  changeset 0488cf67d707 

I also added the option to specify masks as "/yy" string notation in the constructor.  changeset dc159c3d34ee