DragonFly BSD
DragonFly kernel List (threaded) for 2004-01
[Date Prev][Date Next]  [Thread Prev][Thread Next]  [Date Index][Thread Index]

Re: call for API review: new bitstring functions

To: Max Laier <max@xxxxxxxxxxxxxx>
From: Jeffrey Hsu <hsu@xxxxxxxxxxx>
Date: Fri, 02 Jan 2004 21:45:35 -0800

Max Laier wrote:

AFAIR, bitstring is "macro centric" and uses an additional reference
 to return values, you'd like to keep that?!

Actually, no. I think it's bad design to reflect artifacts of the
implementation in the API. But I do understand that the orginal
designers chose, at the time, to compromise the API in order to gain
efficiency by implementing the functions as macros. However, now that
GCC has extended macros to return a value and now that inline functions
are in the C language standard, there's no need to make that compromise in
the API design anymore. Plus, the new functions are too big to be macros. The
same size argument could be made for bit_nclear(), bit_nset(), bit_ffs(), and bit_ffc(),
especially if the compiler unrolls the inner loop. So, for the new
functions, I purposely chosed to return a value than to store into a possibly
invalid pointer. But I do agree that if we were to add the new functions to the
bitstring API, we should following the existing convention of passing a pointer to
the return result.

Apart from that I dislike the naming a bit and would rather go for:
bit_rffs(), bit_rfls() ... as it is already used for the bit_n*-class

Yes, this would be the consistent thing to do. But I just have a hard time remembering what rffs and rfls mean. And I wrote them!

I find your consistency argument to be compelling. In light of the
consistency argument, I think it would be better not to shoehorn these
new functions into the existhing bitstring API. The new plan is to name
the functions bitrange_firstset() and bitrange_lastset() and either keep them internal
to the one file where they're going to be used or to declare them in <sys/bitrange.h>.



[Date Prev][Date Next]  [Thread Prev][Thread Next]  [Date Index][Thread Index]