On Tue, Jan 17, 2012 at 12:51:17PM -0800, Ethan Jackson wrote: > > Bit 0 of 'src' is the bit with value 1 in > > + * src[src_len - 1], bit 1 is the bit with value 2, bit 8 is the bit with > > value > > + * 1 in src[src_len - 2], and so on, and similarly for 'dst'. > > I find this bit of the comments a bit confusing. My understanding, is > that you are basically saying that the data is in network byte order?
Yes. > If so it may be worth stating that explicitly? The reason that I did not is because not everyone agrees on bit numbering. If I recall correctly, IBM mainframe people like to number the most-significant bit (rather than least-significant bit) within a unit as bit 0. I think those people are crazy, but I got tired of getting jumped all over in comp.lang.c about it, and now I'm very specific about this stuff. > When you use the word 'value' in this comment do you perhaps mean > 'index' instead? No. A bit has a value based on its position, and that's what I mean. My goal was to be clear, but I can see that I wasn't. How about this wording instead: * If you consider all of 'src' to be a single unsigned integer in network byte * order, then bit N is the bit with value 2**N. That is, bit 0 is the bit * with value 1 in src[src_len - 1], bit 1 is the bit with value 2, bit 2 is * the bit with value 4, ..., bit 8 is the bit with value 1 in src[src_len - * 2], and so on. Similarly for 'dst'. > > + * Required invariants: > > + * ? src_ofs + n_bits <= src_len * 8 > > + * ? dst_ofs + n_bits <= dst_len * 8 > > + * ? 'src' and 'dst' must not overlap. > > I think it would be trivial to assert these invariants. Seems worth it to me. I don't want to do so in this case because it could greatly increase the cost of the function in cases where it is very cheap. > Do you expect much churn in this code? If so would it be worth unit > testing? I don't feel strongly about it. It's probably worth unit testing in any case. I have no faith in it at all. I'll work on that now while we debate the stuff above. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
