[ccan] [PATCH] mem: module for common operations on addr+length regions

[Rusty Russell]

Cody P Schafer <dev at codyps.com> writes:
> Impliments 'mem' variants of <string.h> and <ccan/str/str.h> functions.
>
> Signed-off-by: Cody P Schafer <dev at codyps.com>

I see David's point about bytestring overlap, but agree also it makes
sense to have these.

And yes, roll in ccan/memmem, too.

I've given a thorough nit-picking below.  Ignore at your leisure :)

> +/**
> + * mem - provide various functions for handling memory (very similar to str for strings)

I get what you mean, but how about the punchier:
        mem - functions for handling memory (like ccan/str but for bytes)

> +#include "config.h"
> +
> +#include <string.h>
> +#include <stdbool.h>
> +#include <ccan/mem/mem.h>
> +
> +size_t memspn(const char *data, size_t len, const char *accept, size_t accept_len)
> +{
> +	size_t i, j;
> +	for (i = 0; i < len; i++) {
> +		for (j = 0; j < accept_len; j++)
> +			if (accept[j] == data[i])
> +				goto cont_outer;
> +		break;
> +cont_outer:
> +		;
> +	}
> +	return i;
> +}

The inner loop is memchr, isn't it?  And with many others...

> +void *memcchr(void const *data, int c, size_t data_len)
> +{
> +	char const *p = data;
> +	while((size_t)(p - ((char const *)data)) < data_len) {

The cast to size_t is a bit weird.


> +char *mempbrk(const char *data, size_t len, const char *accept, size_t accept_len);

Why are these char *?  Maybe that makes sense...

> +static inline bool memends(const char *m, size_t m_len, const char *postfix, size_t postfix_len)

But I don't think this one does.  I can imagine people comparing the end
of anything...

But basically, it's really nicely done!

Have you seen what I did with CCAN_STR_DEBUG() and typesafety?  It
might be a really nice thing to have for these.

In particular, making sure that you're not accidentally casting away
const via a mem* function (which needs __typeof__, see "GNU magic" at
the bottom of str.h), or dealing with two completely incompatible types,
eg:

/* a and be should be comparable: one void or the same type */
#define memeq(a, al, b, bl)
        (memeq_((a), (al), (b), (bl) + 0*sizeof((a) == (b)))

static inline bool memeq_(const void *a, size_t al, const void *b, size_t bl)
...

Sure, sometimes you'll really want to do that, but then a cast to void *
is probably preferable to letting weird mistakes through.

(The final litmas test is usage; if you have users for this see what
that change does to them...)

Cheers,
Rusty.