Re: [PATCH 5/7] tmp-objdir: new API for creating and removing primary object dirs

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Tue, Aug 31 2021, Elijah Newren via GitGitGadget wrote:

I commented just now on how this API is duplicated between here &
another in-flight series in
https://lore.kernel.org/git/87v92lxhh4.fsf@evledraar.gmail.com/; Just
comments on this patch here:

> diff --git a/tmp-objdir.c b/tmp-objdir.c
> index b8d880e3626..9f75a75d1c0 100644
> --- a/tmp-objdir.c
> +++ b/tmp-objdir.c
> @@ -288,7 +288,36 @@ const char **tmp_objdir_env(const struct tmp_objdir *t)
>  	return t->env.v;
>  }
>  
> +const char *tmp_objdir_path(struct tmp_objdir *t)
> +{
> +	return t->path.buf;
> +}

Not your fault & this pre-dates your patch, but FWIW I prefer our APIs
that don't have these "hidden struct" shenanigans (like say "struct
string_list") so a caller could just access this, we can just declare it
"const" appropriately.

We're also all in-tree friends here, so having various accessors for no
reason other than to access struct members seems a bit too much.

All of which is to say if you + Neeraj come up with some common API I
for one wouldn't mind moving the struct deceleration to tmp-objdir.h,
but it looks like in his version it can stay "private" more easily.

In this particular case I hadn't understood on a previous reading of
tmp-objdir.c why this "path" isn't a statically allocated "char
path[PATH_MAX]", or a least we that hardcoded "1024" should be a
PATH_MAX, as it is in some other cases.

But I digress :)