Re: [PATCH v6 1/8] bitops: Introduce the for_each_set_clump8 macro

[Rasmus Villemoes]

On 19/12/2018 07.00, William Breathitt Gray wrote:
> +/**
> + * bitmap_set_value8 - set an 8-bit value within a memory region
> + * @bitmap: address to the bitmap memory region
> + * @value: the 8-bit value
> + * @start: bit offset of the 8-bit value
> + */
> +void bitmap_set_value8(unsigned long *const bitmap,
> +		       const unsigned long *const value,
> +		       const unsigned int start)
> +{
> +	const size_t index = BIT_WORD(start);
> +	const unsigned int offset = start % BITS_PER_LONG;
> +	const unsigned long mask = GENMASK(7, offset);
> +
> +	bitmap[index] &= ~mask;
> +	bitmap[index] |= (*value << offset) & mask;
> +}

errh, why pass the value by reference? That seems rather odd. Also, if
anybody passes a value that doesn't fit in 8 bits, they get what they
deserve; IOW I don't see a reason for the masking in the last line, but
if its convenient for the users, keep it. In any case, please change the
type of value.

Rasmus