On Mon, Oct 04 2021, Elijah Newren wrote:
> On Sat, Oct 2, 2021 at 2:07 AM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote:
>>
>> On Fri, Oct 01 2021, Elijah Newren wrote:
>>
> ...
>> > So maybe I'll submit some patches on top that rip these direct members
>> > out of of unpack_trees_options and push them inside some opaque
>> > struct.
>>
>> Sure, that sounds good. I only had a mild objection to doing it in a way
>> where you'll need that sort of code I removed in the linked commit in
>> prep_exclude() because you were trying not to expose that at any cost,
>> including via some *_INIT macro. I.e. if it's private we can just name
>> it "priv_*" or have a :
>>
>> struct dont_touch_this {
>> struct dir_struct dir;
>> };
>>
>> Which are both ways of /messaging/ that it's private, and since the
>> target audience is just the rest of the git.git codebase I think that
>> ultimately something that 1) sends the right message 2) makes accidents
>> pretty much impossible suffices. I.e. you don't accidentally introduce a
>> new API user accessing a field called "->priv_*" or
>> "->private_*". Someone will review those patches...
>
> An internal struct with all the members meant to be internal-only
> provides nearly all the advantages that I was going for with the
> opaque struct, while also being a smaller change than what I was
> thinking of doing. I like that idea; thanks for the suggestion.
Yeah, just to provide an explicit example something like the below. It
compiles to the same assembly (at least under -O3, didn't exhaustively
try other optimization levels).
I'm rather "meh" on it v.s. just prefixing the relevant member names
with "priv_" or "private_", but it results in the same semantics &
machine code, so it's effectively just a way of doing the labeling for
human consumption.
diff --git a/dir.c b/dir.c
index 39fce3bcba7..a714640e782 100644
--- a/dir.c
+++ b/dir.c
@@ -1533,12 +1533,12 @@ static void prep_exclude(struct dir_struct *dir,
* which originate from directories not in the prefix of the
* path being checked.
*/
- while ((stk = dir->exclude_stack) != NULL) {
+ while ((stk = dir->private.exclude_stack) != NULL) {
if (stk->baselen <= baselen &&
!strncmp(dir->basebuf.buf, base, stk->baselen))
break;
- pl = &group->pl[dir->exclude_stack->exclude_ix];
- dir->exclude_stack = stk->prev;
+ pl = &group->pl[dir->private.exclude_stack->exclude_ix];
+ dir->private.exclude_stack = stk->prev;
dir->pattern = NULL;
free((char *)pl->src); /* see strbuf_detach() below */
clear_pattern_list(pl);
@@ -1584,7 +1584,7 @@ static void prep_exclude(struct dir_struct *dir,
base + current,
cp - base - current);
}
- stk->prev = dir->exclude_stack;
+ stk->prev = dir->private.exclude_stack;
stk->baselen = cp - base;
stk->exclude_ix = group->nr;
stk->ucd = untracked;
@@ -1605,7 +1605,7 @@ static void prep_exclude(struct dir_struct *dir,
dir->pattern->flags & PATTERN_FLAG_NEGATIVE)
dir->pattern = NULL;
if (dir->pattern) {
- dir->exclude_stack = stk;
+ dir->private.exclude_stack = stk;
return;
}
}
@@ -1662,7 +1662,7 @@ static void prep_exclude(struct dir_struct *dir,
invalidate_gitignore(dir->untracked, untracked);
oidcpy(&untracked->exclude_oid, &oid_stat.oid);
}
- dir->exclude_stack = stk;
+ dir->private.exclude_stack = stk;
current = stk->baselen;
}
strbuf_setlen(&dir->basebuf, baselen);
@@ -3302,7 +3302,7 @@ void dir_clear(struct dir_struct *dir)
free(dir->ignored);
free(dir->entries);
- stk = dir->exclude_stack;
+ stk = dir->private.exclude_stack;
while (stk) {
struct exclude_stack *prev = stk->prev;
free(stk);
diff --git a/dir.h b/dir.h
index 83f46c0fb4c..d30d294308d 100644
--- a/dir.h
+++ b/dir.h
@@ -209,6 +209,11 @@ struct untracked_cache {
* record the paths discovered. A single `struct dir_struct` is used regardless
* of whether or not the traversal recursively descends into subdirectories.
*/
+
+struct dir_struct_private {
+ struct exclude_stack *exclude_stack;
+};
+
struct dir_struct {
/* The number of members in `entries[]` array. */
@@ -327,7 +332,7 @@ struct dir_struct {
* (sub)directory in the traversal. Exclude points to the
* matching exclude struct if the directory is excluded.
*/
- struct exclude_stack *exclude_stack;
+ struct dir_struct_private private;
struct path_pattern *pattern;
struct strbuf basebuf;
