[RFC] Define/use pad() and pad_mask() instead of open coding it

[Matt Turner]

On Thu, Mar 11, 2010 at 8:37 PM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> On Thu, Mar 11, 2010 at 08:19:04PM -0500, Matt Turner wrote:
>> On Thu, Mar 11, 2010 at 8:05 PM, Peter Hutterer
>> <peter.hutterer at who-t.net> wrote:
>> > On Thu, Mar 11, 2010 at 07:33:30PM -0500, Matt Turner wrote:
>> >> à la the Linux Kernel's ALIGN and __ALIGN_MASK macros.
>> >>
>> >> Not even compile tested, some files might need to include "misc.h".
>> >>
>> >> CC: Peter Hutterer <peter.hutterer at who-t.net>
>> >> Signed-off-by: Matt Turner <mattst88 at gmail.com>
>> >
>> > ACK in principle, not tested either. :)
>> >
>> >> diff --git a/include/misc.h b/include/misc.h
>> >> index 62d813e..98d39bd 100644
>> >> --- a/include/misc.h
>> >> +++ b/include/misc.h
>> >> @@ -200,6 +200,16 @@ bytes_to_int32(const int bytes) {
>> >>      return (((bytes) + 3) >> 2);
>> >>  }
>> >
>> > + /**
>> > +  * please comment on what I'm supposed to be doing
>> > +  */
>>
>> Got any suggested text?
>
> nothing special, just something like this:
>
> pad(): return the number of bytes needed to align "bytes" with a multiple of
> "alignment"
> (assuming the second param was renamed of course). that should do, plus the
> param/return description for completeness' sake.
>
> pad_mask(): return the number of bytes needed to align "bytes" with the bit
> mask.

Alright, I have:
/**
 * Calculate the number of bytes needed to align "bytes" with the bit mask.
 * @param bytes The minimum number of bytes needed.
 * @param mask The bit mask to align to.
 * @return The number aligned to the bit mask.
 */
static inline int
pad_mask(const int bytes, const int mask) {
    return ((bytes + mask) & ~mask);
}

/**
 * Calculate the number of bytes needed to align "bytes" to a multiple of
 * "alignment".
 * @param bytes The minimum number of bytes needed.
 * @param alignment The number to align to.
 * @return The next multiple of "alignment" greater or equal to "bytes".
 */
static inline int
pad(const int bytes, const int alignment) {
    return pad_mask(bytes, alignment - 1);
}

How's this?

> it might be worth checking the mask for completeness as well. Or at least
> document what happens when you call a pad_mask(2, 3)?

I'm not sure I follow here. Do you mean that the mask should always be
a power of two minus one?

> also, looks like by adding these you can replace the body of bits_to_bytes
> and friends with pad().

I guess, but
- return (((bytes) + 3) >> 2);
+ return pad(bytes, 4) >> 2;
doesn't look like an improvement to me (looks like an extra and-not)
Maybe I missed it.

Thanks,
Matt

>
> Cheers,
>  Peter
>
>> >>
>> >> +static inline int
>> >> +pad_mask(const int bytes, const int mask) {
>> >> +     return (((bytes) + mask) & ~mask);
>> >
>> > you don't need the () around bytes, this isn't a macro.
>> > which, looking at the context goes for bytes_to_int32 as well :)
>> >
>> >> +}
>> >> +
>> >> +static inline int
>> >> +pad(const int bytes, const int mask) {
>> >
>> > shouldn't the second arg be called alignment or something, not mask?
>> > otherwise, it's a bit confusing why both pad() and pad_mask() take a mask
>> > argument.
>>
>> Good point.
>>
>> >> +     return pad_mask(bytes, mask - 1);
>> >> +}
>> >> +
>> >
>> > Cheers,
>> >  Peter
>>
>> I've found some more too--((x + bytes - 1) / bytes) * bytes. Ugh, how nasty..
>>
>> Matt