* [PATCH] fsck: remove redundant parse_tree() invocation
@ 2017-07-18 22:28 Jonathan Tan
2017-07-19 23:25 ` Jonathan Nieder
0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Tan @ 2017-07-18 22:28 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
If obj->type == OBJ_TREE, an invocation of fsck_walk() will invoke
parse_tree() and return quickly if that returns nonzero, so it is of no
use for traverse_one_object() to invoke parse_tree() in this situation
before invoking fsck_walk(). Remove that code.
The behavior of traverse_one_object() is changed slightly in that it now
returns -1 instead of 1 in the case that parse_tree() fails, but this is
not an issue because its only caller (traverse_reachable) does not care
about the value as long as it is nonzero.
This code was introduced in commit 271b8d2 ("builtin-fsck: move away
from object-refs to fsck_walk", 2008-02-25). The same issue existed in
that commit.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
Here's a code cleanup. I noticed this while looking at modifying fsck.
---
builtin/fsck.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 99dea7adf..4ba311cda 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -168,18 +168,7 @@ static void mark_object_reachable(struct object *obj)
static int traverse_one_object(struct object *obj)
{
- int result;
- struct tree *tree = NULL;
-
- if (obj->type == OBJ_TREE) {
- tree = (struct tree *)obj;
- if (parse_tree(tree) < 0)
- return 1; /* error already displayed */
- }
- result = fsck_walk(obj, obj, &fsck_walk_options);
- if (tree)
- free_tree_buffer(tree);
- return result;
+ return fsck_walk(obj, obj, &fsck_walk_options);
}
static int traverse_reachable(void)
--
2.13.2.932.g7449e964c-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] fsck: remove redundant parse_tree() invocation
2017-07-18 22:28 Jonathan Tan
@ 2017-07-19 23:25 ` Jonathan Nieder
0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Nieder @ 2017-07-19 23:25 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git
Hi,
Jonathan Tan wrote:
> If obj->type == OBJ_TREE, an invocation of fsck_walk() will invoke
> parse_tree() and return quickly if that returns nonzero, so it is of no
> use for traverse_one_object() to invoke parse_tree() in this situation
> before invoking fsck_walk(). Remove that code.
I like the diffstat. I'm trying to figure out what the old code was
trying to do, since that will make it easier to understand this
change.
fsck_walk_tree calls parse_tree, so why did we need to parse it in
advance? Was it just about handling the error differently when it
fails to parse?
It appears this code comes from
https://public-inbox.org/git/20080214090013.GK24004@spearce.org/:
+ if (parse_tree(item) < 0)
+ return; /* error already displayed */
+
+ init_tree_desc(&desc, item->buffer, item->size);
At that point the parse_tree call was not redundant. Later it must
have been amended and cleaned up to use fsck_walk.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks for cleaning up.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fsck: remove redundant parse_tree() invocation
@ 2023-08-02 0:21 กัมปนาท ทุมวงศ์
0 siblings, 0 replies; 4+ messages in thread
From: กัมปนาท ทุมวงศ์ @ 2023-08-02 0:21 UTC (permalink / raw)
To: jonathantanmy; +Cc: git
ส่งจาก iPhone ของฉัน
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fsck: remove redundant parse_tree() invocation
@ 2023-08-02 0:28 กัมปนาท ทุมวงศ์
0 siblings, 0 replies; 4+ messages in thread
From: กัมปนาท ทุมวงศ์ @ 2023-08-02 0:28 UTC (permalink / raw)
To: jonathantanmy; +Cc: git
ส่งจาก iPhone ของฉัน
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-08-02 0:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-02 0:28 [PATCH] fsck: remove redundant parse_tree() invocation กัมปนาท ทุมวงศ์
-- strict thread matches above, loose matches on Subject: below --
2023-08-02 0:21 กัมปนาท ทุมวงศ์
2017-07-18 22:28 Jonathan Tan
2017-07-19 23:25 ` Jonathan Nieder
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).