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

From: Jonathan Nieder <[email-masked]>
Date: Thu, 1 Oct 2020 21:12:14 -0700

Han-Wen Nienhuys wrote:

> --- /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?

[...]
> +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?

[...]
> +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