* [PATCH] Remove redundant double exclamation points
@ 2022-12-19 14:12 Rose via GitGitGadget
2022-12-19 15:19 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 3+ messages in thread
From: Rose via GitGitGadget @ 2022-12-19 14:12 UTC (permalink / raw)
To: git; +Cc: Rose, Seija Kijin
From: Seija Kijin <doremylover123@gmail.com>
S_ISDIR is a macro that involves a "==" comparison.
This means the !! is redundant and not needed.
Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
Remove redundant double exclamation points
S_ISDIR is a macro that involves a "==" comparison.
This means the !! is redundant and not needed.
Signed-off-by: Seija Kijin doremylover123@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1401%2FAtariDreams%2FIS_DIR-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1401/AtariDreams/IS_DIR-v1
Pull-Request: https://github.com/git/git/pull/1401
tree-walk.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/tree-walk.c b/tree-walk.c
index 74f4d710e8f..6b51d27ccb2 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -1040,9 +1040,9 @@ static enum interesting do_match(struct index_state *istate,
ps->max_depth == -1)
return all_entries_interesting;
return within_depth(base->buf + base_offset, baselen,
- !!S_ISDIR(entry->mode),
- ps->max_depth) ?
- entry_interesting : entry_not_interesting;
+ S_ISDIR(entry->mode), ps->max_depth) ?
+ entry_interesting :
+ entry_not_interesting;
}
pathlen = tree_entry_len(entry);
@@ -1073,8 +1073,7 @@ static enum interesting do_match(struct index_state *istate,
if (within_depth(base_str + matchlen + 1,
baselen - matchlen - 1,
- !!S_ISDIR(entry->mode),
- ps->max_depth))
+ S_ISDIR(entry->mode), ps->max_depth))
goto interesting;
else
return entry_not_interesting;
base-commit: 7c2ef319c52c4997256f5807564523dfd4acdfc7
--
gitgitgadget
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] Remove redundant double exclamation points
2022-12-19 14:12 [PATCH] Remove redundant double exclamation points Rose via GitGitGadget
@ 2022-12-19 15:19 ` Ævar Arnfjörð Bjarmason
2022-12-20 0:50 ` Junio C Hamano
0 siblings, 1 reply; 3+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-19 15:19 UTC (permalink / raw)
To: Rose via GitGitGadget; +Cc: git, Seija Kijin
On Mon, Dec 19 2022, Rose via GitGitGadget wrote:
> From: Seija Kijin <doremylover123@gmail.com>
>
> S_ISDIR is a macro that involves a "==" comparison.
It does? The POSIX standard
(https://pubs.opengroup.org/onlinepubs/009604499/basedefs/sys/stat.h.html)
says:
The following macros shall be provided to test whether a file is
of the specified type. The value m supplied to the macros is the
value of st_mode from a stat structure. The macro shall evaluate
to a non-zero value if the test is true; 0 if the test is false.
The "non-zero" there seems to intentionally leave open that this may be
defined e.g. as via a "&" test, as opposed to "==" which according to
C99's 6.5.9.3 says:
The == (equal to) and != (not equal to) operators are analogous
to the relational operators except for their lower
precedence.90) Each of the operators yields 1 if the specified
relation is true and 0 if it is false. The result has type
int. For any pair of operands, exactly one of the relations is
true.
> This means the !! is redundant and not needed.
I think you're therefore introducing a bug here, this may work on your
platform, but we have no guarantee that it'll work elsewhere.
I thought that it probably wouldn't matter, as we'd treat the argument
as a boolean, but we don't. In within_depth() we proceed to use the
passed-in 3rd argument (depth) as a counter, so it really does matter if
it's 0, 1, or other non-zero.
> tree-walk.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/tree-walk.c b/tree-walk.c
> index 74f4d710e8f..6b51d27ccb2 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -1040,9 +1040,9 @@ static enum interesting do_match(struct index_state *istate,
> ps->max_depth == -1)
> return all_entries_interesting;
> return within_depth(base->buf + base_offset, baselen,
> - !!S_ISDIR(entry->mode),
> - ps->max_depth) ?
> - entry_interesting : entry_not_interesting;
> + S_ISDIR(entry->mode), ps->max_depth) ?
> + entry_interesting :
> + entry_not_interesting;
> }
>
> pathlen = tree_entry_len(entry);
Aside from whether or not this is a bug, could you please submit
proposed refactorings of the git project via coccinelle patches if
possible (as I suggested to you before).
I realize that it has a slight learning curve, but it makes writing &
maintaining these so much easier, and it'll fix (mis)uses going forward,
not just as a one-off.
So, as an example (and assuming this wasn't buggy), you'd do that in
this case as e.g. (untested, but you can see similar syntax in our
existing *.cocci files):
@@
@@
- !!
(
S_ISDIR
|
S_ISFIFO
// |
// we'd continue to list the rest here...
)
(...)
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-12-20 0:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-19 14:12 [PATCH] Remove redundant double exclamation points Rose via GitGitGadget
2022-12-19 15:19 ` Ævar Arnfjörð Bjarmason
2022-12-20 0:50 ` Junio C Hamano
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 read-only IMAP folder(s) and NNTP newsgroup(s).