Bart, I'm interested to look at the patch. But note that for consistency, Packet::set_ip_header_anno() would probably imply that that one accesses the IP header via Packet::ip_header_anno(), and so forth. We currently use Packet::ip_header(), which is better....
Eddie Bart Braem wrote: > Hi Eddie, > > On 31 Oct 2008, at 01:37, Eddie Kohler wrote: > >> Bart Braem wrote: >>> Students were doing exercise where they had to change an IP header >>> of a packet. What they did was use ip_header to get the current >>> ip_header, take a copy of that struct, modify it and then set it with >>> set_ip_header. And that did not work. >>> We were also confused in the beginning, until we realised the >>> semantics of set_ip_header: it only sets the pointers and it does >>> not copy anything. In fact, do we understand correctly that the >>> set_ip_header should be treated as an annotation function and that >>> the pointer set with it should lie within the packet boundaries? >> >> That's exactly correct. In fact, since commit >> f03d536cc8392f673fb90da298ebbacb77eb3ca1, we assert that the pointer >> is in range. >> > > I did not notice that patch, the assertion certainly makes sense there. > Thanks! > > >>> We suggest to update the network header methods to better reflect >>> these semantics. >>> We could introduce naming like set_ip_header_anno. >>> But we also suggest to change the call itself and use an offset >>> parameter instead of a pointer to the data. This way confusion is >>> impossible because it is clear that the network_header methods work >>> on in-packet data. >>> What do you think? >> >> I see what you mean, but don't you think the assertion is enough? >> Perhaps a name change to _anno() would be useful, but set_ip_header() >> is very well established by this point. My tendency is to leave >> things as they are. > > > In my opinion a name change would be better, but I do understand that > then also similar functions for all other headers would have to be > changed as well to stay consistent. Which is quite a large job, but I am > willing to create the patch myself. > It will be a large job, but it can only improve the internal consistency > and the accessibility of Click to new users. > > Regards, > Bart _______________________________________________ click mailing list [email protected] https://amsterdam.lcs.mit.edu/mailman/listinfo/click
