Re: [PATCH v2 05/13] reftable: utility functions

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 2, 2020, 04:12 UTC
Message-ID: <20201002041214.GE3252492@google.com>
In-Reply-To: <4190da597e65bce072fa3c37c9410a56def4b489.1601568663.git.gitgitgadget@gmail.com>

Han-Wen Nienhuys wrote:

Show 12 quoted lines

> --- /dev/null
> +++ b/reftable/basics.c
> @@ -0,0 +1,131 @@
> +/*
> +Copyright 2020 Google LLC
> +
> +Use of this source code is governed by a BSD-style
> +license that can be found in the LICENSE file or at
> +https://developers.google.com/open-source/licenses/bsd
> +*/
> +
> +#include "basics.h"

Git's source files start with #include-ing "git-compat-util.h"; how do we approach that here?

E.g. would we pass -include on the command line, or could we #include some kind of compat-util.h that #include-s "git-compat-util.h" in Git and does the appropriate steps for enabling feature test macros and including system headers when used outside?

[...]

> +int binsearch(size_t sz, int (*f)(size_t k, void *args), void *args)

How does this compare to stdlib's bsearch?

[...]

Show 12 quoted lines

> +void free_names(char **a)
> +{
> +	char **p = a;
> +	if (p == NULL) {
> +		return;
> +	}
> +	while (*p) {
> +		reftable_free(*p);
> +		p++;
> +	}
> +	reftable_free(a);
> +}

Are there other callers that need custom free?

[...]

Show 10 quoted lines

> +int names_length(char **names)
> +{
> +	int len = 0;
> +	char **p = names;
> +	while (*p) {
> +		p++;
> +		len++;
> +	}
> +	return len;
> +}

The rest are probably easier to evaluate when I look at the callers, and it's time for me to go to sleep now. I'll try to find some time soon to pick the review back up.

Thanks for a thoughtfully put together library so far.

Jonathan