From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Elijah Newren <newren@gmail.com>
Cc: Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>,
Git Mailing List <git@vger.kernel.org>,
Fedor Biryukov <fedor.birjukov@gmail.com>,
Philip Oakley <philipoakley@iee.email>,
Phillip Wood <phillip.wood123@gmail.com>
Subject: Re: [PATCH v3 04/11] unpack-trees: introduce preserve_ignored to unpack_trees_options
Date: Mon, 04 Oct 2021 16:07:31 +0200 [thread overview]
Message-ID: <8735pgop57.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <CABPp-BFE_5zPCZY8adJSQchdQzxq3uH-oma-S=_Sw0OUXx03OQ@mail.gmail.com>
On Mon, Oct 04 2021, Elijah Newren wrote:
> On Sat, Oct 2, 2021 at 2:07 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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;
next prev parent reply other threads:[~2021-10-04 14:13 UTC|newest]
Thread overview: 82+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-18 23:15 [PATCH 0/6] Fix various issues around removal of untracked files/directories Elijah Newren via GitGitGadget
2021-09-18 23:15 ` [PATCH 1/6] t2500: add various tests for nuking untracked files Elijah Newren via GitGitGadget
2021-09-19 13:44 ` Ævar Arnfjörð Bjarmason
2021-09-20 14:48 ` Elijah Newren
2021-09-18 23:15 ` [PATCH 2/6] Split unpack_trees 'reset' flag into two for untracked handling Elijah Newren via GitGitGadget
2021-09-19 13:48 ` Ævar Arnfjörð Bjarmason
2021-09-20 15:20 ` Elijah Newren
2021-09-20 10:19 ` Phillip Wood
2021-09-20 16:05 ` Elijah Newren
2021-09-20 18:11 ` Phillip Wood
2021-09-24 2:27 ` Elijah Newren
2021-09-18 23:15 ` [PATCH 3/6] unpack-trees: avoid nuking untracked dir in way of unmerged file Elijah Newren via GitGitGadget
2021-09-18 23:15 ` [PATCH 4/6] unpack-trees: avoid nuking untracked dir in way of locally deleted file Elijah Newren via GitGitGadget
2021-09-19 13:52 ` Ævar Arnfjörð Bjarmason
2021-09-20 16:12 ` Elijah Newren
2021-09-18 23:15 ` [PATCH 5/6] Comment important codepaths regarding nuking untracked files/dirs Elijah Newren via GitGitGadget
2021-09-24 11:47 ` Luke Diamand
2021-09-24 13:41 ` Elijah Newren
2021-09-18 23:15 ` [PATCH 6/6] Documentation: call out commands that nuke untracked files/directories Elijah Newren via GitGitGadget
2021-09-19 10:52 ` Philip Oakley
2021-09-19 13:36 ` Philip Oakley
2021-09-20 16:29 ` Elijah Newren
2021-09-24 6:37 ` [PATCH v2 0/6] Fix various issues around removal of " Elijah Newren via GitGitGadget
2021-09-24 6:37 ` [PATCH v2 1/6] t2500: add various tests for nuking untracked files Elijah Newren via GitGitGadget
2021-09-24 6:37 ` [PATCH v2 2/6] Change unpack_trees' 'reset' flag into an enum Elijah Newren via GitGitGadget
2021-09-24 17:35 ` Junio C Hamano
2021-09-26 6:50 ` Elijah Newren
2021-09-24 6:37 ` [PATCH v2 3/6] unpack-trees: avoid nuking untracked dir in way of unmerged file Elijah Newren via GitGitGadget
2021-09-24 6:37 ` [PATCH v2 4/6] unpack-trees: avoid nuking untracked dir in way of locally deleted file Elijah Newren via GitGitGadget
2021-09-24 6:37 ` [PATCH v2 5/6] Comment important codepaths regarding nuking untracked files/dirs Elijah Newren via GitGitGadget
2021-09-24 17:50 ` Eric Sunshine
2021-09-26 6:35 ` Elijah Newren
2021-09-24 6:37 ` [PATCH v2 6/6] Documentation: call out commands that nuke untracked files/directories Elijah Newren via GitGitGadget
2021-09-27 16:33 ` [PATCH v3 00/11] Fix various issues around removal of " Elijah Newren via GitGitGadget
2021-09-27 16:33 ` [PATCH v3 01/11] t2500: add various tests for nuking untracked files Elijah Newren via GitGitGadget
2021-09-27 16:33 ` [PATCH v3 02/11] checkout, read-tree: fix leak of unpack_trees_options.dir Elijah Newren via GitGitGadget
2021-09-27 16:33 ` [PATCH v3 03/11] read-tree, merge-recursive: overwrite ignored files by default Elijah Newren via GitGitGadget
2021-12-13 17:12 ` Jack O'Connor
2021-12-13 20:10 ` Elijah Newren
2021-09-27 16:33 ` [PATCH v3 04/11] unpack-trees: introduce preserve_ignored to unpack_trees_options Elijah Newren via GitGitGadget
2021-09-29 9:22 ` Ævar Arnfjörð Bjarmason
2021-09-29 15:35 ` Elijah Newren
2021-09-29 18:30 ` Ævar Arnfjörð Bjarmason
2021-09-30 4:25 ` Elijah Newren
2021-09-30 14:04 ` Ævar Arnfjörð Bjarmason
2021-10-01 1:53 ` Elijah Newren
2021-10-01 8:15 ` Ævar Arnfjörð Bjarmason
2021-10-01 9:53 ` Ævar Arnfjörð Bjarmason
2021-10-01 18:50 ` Elijah Newren
2021-10-02 8:44 ` Ævar Arnfjörð Bjarmason
2021-10-03 22:21 ` Ævar Arnfjörð Bjarmason
2021-10-04 13:45 ` Elijah Newren
2021-10-04 13:45 ` Elijah Newren
2021-10-04 14:07 ` Ævar Arnfjörð Bjarmason [this message]
2021-10-04 14:57 ` Elijah Newren
2021-09-27 16:33 ` [PATCH v3 05/11] unpack-trees: make dir an internal-only struct Elijah Newren via GitGitGadget
2021-09-27 16:33 ` [PATCH v3 06/11] Remove ignored files by default when they are in the way Elijah Newren via GitGitGadget
2021-09-27 16:33 ` [PATCH v3 07/11] Change unpack_trees' 'reset' flag into an enum Elijah Newren via GitGitGadget
2021-09-27 16:33 ` [PATCH v3 08/11] unpack-trees: avoid nuking untracked dir in way of unmerged file Elijah Newren via GitGitGadget
2021-09-27 16:33 ` [PATCH v3 09/11] unpack-trees: avoid nuking untracked dir in way of locally deleted file Elijah Newren via GitGitGadget
2021-09-27 16:33 ` [PATCH v3 10/11] Comment important codepaths regarding nuking untracked files/dirs Elijah Newren via GitGitGadget
2021-09-27 16:33 ` [PATCH v3 11/11] Documentation: call out commands that nuke untracked files/directories Elijah Newren via GitGitGadget
2021-09-27 20:36 ` [PATCH v3 00/11] Fix various issues around removal of " Junio C Hamano
2021-09-27 20:41 ` Elijah Newren
2021-09-27 21:31 ` Elijah Newren
2021-09-30 14:00 ` Phillip Wood
[not found] ` <aaa8ea3b-0902-f9e6-c1a4-0ca2b1b2f57b@gmail.com>
2021-10-01 2:08 ` Elijah Newren
2021-10-04 1:11 ` [RFC PATCH v4 00/10] " Ævar Arnfjörð Bjarmason
2021-10-04 1:11 ` [RFC PATCH v4 01/10] t2500: add various tests for nuking untracked files Ævar Arnfjörð Bjarmason
2021-10-04 1:11 ` [RFC PATCH v4 02/10] read-tree, merge-recursive: overwrite ignored files by default Ævar Arnfjörð Bjarmason
2021-10-04 1:11 ` [RFC PATCH v4 03/10] unpack-trees: introduce preserve_ignored to unpack_trees_options Ævar Arnfjörð Bjarmason
2021-10-04 1:11 ` [RFC PATCH v4 04/10] unpack-trees: rename "dir" to "private_dir" Ævar Arnfjörð Bjarmason
2021-10-04 1:11 ` [RFC PATCH v4 05/10] Remove ignored files by default when they are in the way Ævar Arnfjörð Bjarmason
2021-10-04 1:11 ` [RFC PATCH v4 06/10] Change unpack_trees' 'reset' flag into an enum Ævar Arnfjörð Bjarmason
2021-10-04 1:11 ` [RFC PATCH v4 07/10] unpack-trees: avoid nuking untracked dir in way of unmerged file Ævar Arnfjörð Bjarmason
2021-10-04 1:11 ` [RFC PATCH v4 08/10] unpack-trees: avoid nuking untracked dir in way of locally deleted file Ævar Arnfjörð Bjarmason
2021-10-04 1:11 ` [RFC PATCH v4 09/10] Comment important codepaths regarding nuking untracked files/dirs Ævar Arnfjörð Bjarmason
2021-10-04 1:11 ` [RFC PATCH v4 10/10] Documentation: call out commands that nuke untracked files/directories Ævar Arnfjörð Bjarmason
2021-10-04 14:38 ` [RFC PATCH v4 00/10] Fix various issues around removal of " Elijah Newren
2021-10-04 16:08 ` Ævar Arnfjörð Bjarmason
2021-10-05 7:40 ` Elijah Newren
2021-10-04 18:17 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8735pgop57.fsf@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=fedor.birjukov@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=newren@gmail.com \
--cc=philipoakley@iee.email \
--cc=phillip.wood123@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).