On Tue, Aug 18, 2015 at 7:08 PM, David Gibson <da...@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 <d...@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? _______________________________________________ ccan mailing list ccan@lists.ozlabs.org https://lists.ozlabs.org/listinfo/ccan