[PATCH] Fix and expand bitvec interface.

Harald Welte laforge at gnumonks.org
Thu Jan 14 10:23:41 UTC 2016


Hi Max,

On Wed, Jan 13, 2016 at 03:24:29PM +0100, suraev at alumni.ntnu.no wrote:
> +int bitvec_set_uint(struct bitvec *bv, unsigned int in, unsigned count);

we generally don't use 'unsigned' in the code so far, but 'unsigned
int'.  So it's a bit odd to see 'unsigned' without int here.  In
general, I probably wouldn't care too much.  However, given that it is a
single function that has one 'unsigned int' and one 'unsigned' argument,
it seems a bit odd.

> +		if (0 == i % 8) printf(" ");
> +		switch (bitvec_get_bit_pos(bv, i)) {
> +		case ZERO: printf("0"); break;
> +		case ONE: printf("1"); break;
> +		case L: printf("L"); break;
> +		case H: printf("H"); break;

If you're printing a single character only, using fputc() should be
much more efficient.  I'm not sure if gcc is good to detect that and
replace it by itself.  If yes, no need to change the code.  If not,
better use fputc.

Also, the library should generally not just printf.  For the majority of
the programs running as daemons this doesn't make sense.  

The general way to deal with this is to offer a function that generates
the string, i.e. 'char *bitvec_to_string(bitvec *bv)'.  Since we assume
single-threaded useres only, having a static buffer and always returning
that buffer has so far been the the generally accepted form.  If you
don't like that, offer a bitvec_to_string_r() using a caller-allocated
string, or maybe even better, something that take a talloc context as
first argument, form where the output-string is then allocated.

-- 
- Harald Welte <laforge at gnumonks.org>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)



More information about the baseband-devel mailing list