[ovs-dev] [eviction 01/12] util: Move bitwise_copy() here, add new bitwise functions.

Ben Pfaff blp at nicira.com
Wed Jan 18 00:15:43 UTC 2012


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.



More information about the dev mailing list