Re: [PATCH/WIP 02/11] notes-merge: use opendir/readdir instead of using read_directory()
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:52:19
Nguyễn Thái Ngọc Duy [off-list ref] writes:
notes_merge_commit() only needs to list all entries (non-recursively) under a directory, which can be easily accomplished with opendir/readdir and would be more lightweight than read_directory(). read_directory() is designed to list paths inside a working directory. Using it outside of its scope may lead to undesired effects.
Technically isn't the directory structure this codepath looks at a working tree that has extract of a notes tree commit? Looking at the result of the patch I do not have strong opinions either way, though. It isn't like we care about gitignore or attributes rules in the notes tree, so using read_directory() does feel like an overkill.
quoted hunk
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted> --- notes-merge.c | 45 +++++++++++++++++++++++++++------------------ 1 files changed, 27 insertions(+), 18 deletions(-)diff --git a/notes-merge.c b/notes-merge.c index e9e4199..80d64a2 100644 --- a/notes-merge.c +++ b/notes-merge.c@@ -680,48 +680,57 @@ int notes_merge_commit(struct notes_merge_options *o, * commit message and parents from 'partial_commit'. * Finally store the new commit object SHA1 into 'result_sha1'. */ - struct dir_struct dir; - char *path = xstrdup(git_path(NOTES_MERGE_WORKTREE "/")); - int path_len = strlen(path), i; + DIR *dir; + struct dirent *e; + struct strbuf path = STRBUF_INIT; const char *msg = strstr(partial_commit->buffer, "\n\n"); + int baselen; - OUTPUT(o, 3, "Committing notes in notes merge worktree at %.*s", - path_len - 1, path); + strbuf_addstr(&path, git_path(NOTES_MERGE_WORKTREE)); + OUTPUT(o, 3, "Committing notes in notes merge worktree at %s", path.buf); if (!msg || msg[2] == '\0') die("partial notes commit has empty message"); msg += 2; - memset(&dir, 0, sizeof(dir)); - read_directory(&dir, path, path_len, NULL); - for (i = 0; i < dir.nr; i++) { - struct dir_entry *ent = dir.entries[i]; + dir = opendir(path.buf); + if (!dir) + die_errno("could not open %s", path.buf); + + strbuf_addch(&path, '/'); + baselen = path.len; + while ((e = readdir(dir)) != NULL) { struct stat st; - const char *relpath = ent->name + path_len; unsigned char obj_sha1[20], blob_sha1[20]; - if (ent->len - path_len != 40 || get_sha1_hex(relpath, obj_sha1)) { - OUTPUT(o, 3, "Skipping non-SHA1 entry '%s'", ent->name); + if (is_dot_or_dotdot(e->d_name)) + continue; + + if (strlen(e->d_name) != 40 || get_sha1_hex(e->d_name, obj_sha1)) { + OUTPUT(o, 3, "Skipping non-SHA1 entry '%s%s'", path.buf, e->d_name); continue; } + strbuf_addstr(&path, e->d_name); /* write file as blob, and add to partial_tree */ - if (stat(ent->name, &st)) - die_errno("Failed to stat '%s'", ent->name); - if (index_path(blob_sha1, ent->name, &st, HASH_WRITE_OBJECT)) - die("Failed to write blob object from '%s'", ent->name); + if (stat(path.buf, &st)) + die_errno("Failed to stat '%s'", path.buf); + if (index_path(blob_sha1, path.buf, &st, HASH_WRITE_OBJECT)) + die("Failed to write blob object from '%s'", path.buf); if (add_note(partial_tree, obj_sha1, blob_sha1, NULL)) die("Failed to add resolved note '%s' to notes tree", - ent->name); + path.buf); OUTPUT(o, 4, "Added resolved note for object %s: %s", sha1_to_hex(obj_sha1), sha1_to_hex(blob_sha1)); + strbuf_setlen(&path, baselen); } create_notes_commit(partial_tree, partial_commit->parents, msg, result_sha1); OUTPUT(o, 4, "Finalized notes merge commit: %s", sha1_to_hex(result_sha1)); - free(path); + strbuf_release(&path); + closedir(dir); return 0; }