Re: [PATCH 5/7] tmp-objdir: new API for creating and removing primary object dirs
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-09-28 08:01:09
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/ (local); Just comments on this patch here:
quoted hunk ↗ jump to hunk
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 :)