Re: [PATCH 1/2] strbuf: add tests

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

I agree with Johannes' remarks. I'd add two style nits:

William Duclot <william.duclot@ensimag.grenoble-inp.fr> writes:

> --- /dev/null
> +++ b/t/helper/test-strbuf.c
> @@ -0,0 +1,69 @@
> +#include "git-compat-util.h"
> +#include "strbuf.h"
> +
> +/*
> + * Check behavior on usual use cases
> + */
> +int test_usual(struct strbuf *sb)
> +{
> +	size_t size, old_alloc;
> +	char *res, *old_buf, *str_test = malloc(5*sizeof(char));

Spaces around binary operator.

> +int main(int argc, char *argv[])
> +{
> +	size_t size = 1;
> +	struct strbuf sb;
> +	char str_test[5] = "test";
> +	char str_foo[7] = "foo";
> +
> +	if (argc != 2)
> +		usage("test-strbuf mode");
> +
> +	if (!strcmp(argv[1], "basic_grow")) {
> +		/*
> +		 * Check if strbuf_grow(0) allocate a new NUL-terminated buffer
> +		 */
> +		strbuf_init(&sb, 0);
> +		strbuf_grow(&sb, 0);
> +		if (sb.buf == strbuf_slopbuf)
> +			die("strbuf_grow failed to alloc memory");
> +		strbuf_release(&sb);
> +		if (sb.buf != strbuf_slopbuf)
> +			die("strbuf_release does not reinitialize the strbuf");
> +	} else if (!strcmp(argv[1], "strbuf_check_behavior")) {
> +		strbuf_init(&sb, 0);
> +		return test_usual(&sb);

I think the command ("strbuf_check_behavior") should have the same name
as the function (test_usual). This avoids one indirection for the
reader going from t*.sh file to the actual test code.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/