[ccan] [PATCH 1/3] mem: add mem helper functions

[Cody P Schafer]

On Tue, Aug 18, 2015 at 7:08 PM, David Gibson
<david at gibson.dropbear.id.au> wrote:
> On Mon, Aug 17, 2015 at 08:33:29PM -0400, Cody P Schafer wrote:
>> Signed-off-by: Cody P Schafer <dev at codyps.com>
>
> Concept looks fine.  A number of minor nits.
>
>> ---
>> +/**
>> + * memstarts - determine if @data starts with @prefix
>> + * @data: does this begin with @prefix?
>> + * @data_len: bytes in @data
>> + * @prefix: does @data begin with these bytes?
>> + * @prefix_len: bytes in @prefix
>> + *
>> + * Returns true if @data starts with @prefix, otherwise return false.
>> + *
>> + * Example:
>> + *   if (memstarts(somebytes, bytes_len, otherbytes, otherbytes_len)) {
>> + *           printf("somebytes starts with otherbytes!\n");
>> + *   }
>> + */
>> +static inline bool memstarts(void const *data, size_t data_len,
>> +             void const *prefix, size_t prefix_len)
>> +{
>> +     if (prefix_len > data_len)
>> +             return false;
>> +     return !memcmp(data, prefix, prefix_len);
>
> I'd prefer to see the order changed, and use memeq() here.

I don't think memeq() fits here, as it includes the length comparison,
which is required when checking if 2 blocks of memory are equal, but
serves no purpose here.

I'd need to either:
 - pass prefix_len twice (ugly)
 - remove the memstarts length check and use MIN(prefix_len,
data_len), which makes it harder to think about the function, and it
seems silly to generate a value we know will cause the function to
return false rather than just returning false directly.

Neither seems really great.

>
>> +}
>> +
>> +/**
>> + * memeq - Are two byte arrays equal?
>> + * @a: first array
>> + * @al: bytes in first array
>> + * @b: second array
>> + * @bl: bytes in second array
>> + *
>> + * Example:
>> + *   if (memeq(somebytes, bytes_len, otherbytes, otherbytes_len)) {
>> + *           printf("memory blocks are the same!\n");
>> + *   }
>> + */
>> +#define memeq(a, al, b, bl) (al == bl && !memcmp(a, b, bl))
>
> Needs brackeds around the macro arguments, in case they're complex
> expressions.

Are you looking for just the following? :

+#define memeq(a, al, b, bl) ((al) == (bl) && !memcmp(a, b, bl))

Or should the function args be paren'd as well?