[ovs-dev] [PATCH ovn v3 2/5] util: Add more bitwise operations.

Ben Pfaff blp at nicira.com
Wed Apr 15 22:20:47 UTC 2015


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 <blp at nicira.com>
> 
> I left a couple of comments, but they're not critical, so:
> 
> Acked-by: Russell Bryant <rbryant at 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.



More information about the dev mailing list