[ccan] Add new 'bytestring' module

[David Gibson]

On Tue, May 22, 2012 at 12:50:04PM +0930, Paul 'Rusty' Russell wrote:
> On Mon, 21 May 2012 11:11:22 +1000, David Gibson <david at gibson.dropbear.id.au> wrote:
> > This patch adds a new 'bytestring' module which manipulates bytestrings
> > consisting of a const char pointer and length.  This makes for more
> > convenient handling of subsections of a large fixed data buffer or buffers.
> > 
> > Signed-off-by: David Gibson <david at gibson.dropbear.id.au>
> 
> Nice!
> 
> Nitpicks below:
> 
> > +/**
> > + * bytestring_eq - test if bytestrings have identical content
> > + * @a, @b: bytestrings
> > + *
> > + * Returns 1 if the given bytestrings have identical length and
> > + * content, 0 otherwise.
> > + */
> > +static inline int bytestring_eq(struct bytestring a, struct bytestring b)
> > +{
> > +	return (a.len == b.len)
> > +		&& (memcmp(a.ptr, b.ptr, a.len) == 0);
> > +}
> 
> bool?

Good idea.

> > diff --git a/ccan/bytestring/test/compile_fail-BYTESTRING.c b/ccan/bytestring/test/compile_fail-BYTESTRING.c
> > new file mode 100644
> > index 0000000..4ebab67
> > --- /dev/null
> > +++ b/ccan/bytestring/test/compile_fail-BYTESTRING.c
> > @@ -0,0 +1,14 @@
> > +#include <stdio.h>
> > +
> > +#include <ccan/bytestring/bytestring.h>
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +	struct bytestring bs;
> > +
> > +#ifdef FAIL
> > +	bs = BYTESTRING(argv[0]);
> > +#endif
> > +	printf("%zd\n", bs.len);
> > +	return 0;
> > +}
> 
> I think with optimization, this will fail due to a warning about bs.len
> being uninitialized.  I prefer to put the FAIL around the minimal
> amount, eg:
> 
>         bs = BYTESTRING(
> #ifdef FAIL
>                         argv[0]
> #else
>                         "literal"
> #endif
>         );

Ah, yes, that's a better idea.

> > +#define TEST_STRING	"test string"
> > +#define TEST_STRING_2	"abc\0def"
> > +
> > +const char str1[] = TEST_STRING;
> > +const char *str2 = TEST_STRING;
> > +
> > +int main(void)
> > +{
> > +	struct bytestring bs, bs1, bs2, bs3;
> > +
> > +	/* This is how many tests you plan to run */
> > +	plan_tests(5);
> > +
> > +	bs = bytestring(str1, sizeof(str1) - 1);
> > +	ok1(bs.ptr == str1);
> > +	ok1(bs.len == (sizeof(str1) - 1));
> > +
> > +	bs1 = BYTESTRING(TEST_STRING);
> > +	ok1(bytestring_eq(bs, bs1));
> > +
> > +	bs2 = BYTESTRING(TEST_STRING_2);
> > +	ok1(bs2.len == 7);
> > +
> > +	bs3 = bytestring_from_string(str2);
> > +	ok1(bytestring_eq(bs3, bs));
> 
> Want to test that bytestring_from_string on a nul-containing string
> does as documented?

Good idea.

Any thoughts on the name of the module?

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson