On Wed, Apr 08, 2015 at 05:27:57PM -0400, Russell Bryant wrote:
> On 04/01/2015 12:52 AM, Ben Pfaff wrote:
> > To be used in upcoming commits.
> >
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
>
> I left a couple of comments, but they're not critical, so:
>
> Acked-by: Russell Bryant <rbry...@redhat.com>
Thanks for the review.
> > +int
> > +bitwise_rscan(const void *p, int len, bool target, int start, int end)
> > +{
>
> Should len by unsigned like len in the rest of the functions added?
That's better, thanks, changed.
> Also, is there a reason not to use size_t for all of them instead?
That would probably be better, yes. Sometimes I use unsigned int
instead of size_t when the size will always be small and I want to keep
a struct small, but that doesn't make sense here.
Until someone goes to the trouble of fixing all of these, I decided to
stick with unsigned int here for consistency.
> > + int ofs;
> > +
> > + for (ofs = start; ofs > end; ofs--) {
> > + if (bitwise_get_bit(p, len, ofs) == target) {
> > + break;
> > + }
> > + }
> > + return ofs;
> > +}
> >
> > /* Copies the 'n_bits' low-order bits of 'value' into the 'n_bits' bits
> > * starting at bit 'dst_ofs' in 'dst', which is 'dst_len' bytes long.
> > @@ -1361,6 +1388,104 @@ bitwise_get(const void *src, unsigned int src_len,
> > n_bits);
> > return ntohll(value);
> > }
> > +
> > +/* Returns the value of the bit with offset 'ofs' in 'ofs', which is 'len'
> > + * bytes long.
>
> Did you mean 'ofs' in 'src' here? The same "'ofs' in 'ofs'" exists in
> other comments, as well.
Ugh. Thanks for pointing that out. Fixed.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
