| Message ID | fbc38ce9075d3d28187351b0fb6b34a27ec431fd.1611397210.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | builtin/ls-files.c:add git ls-file --dedup option | expand |
"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: ZheNing Hu <adlternative@gmail.com> > > This situation may occur in the original code: lstat() failed > but we use `&st` to feed ie_modified() later. > > Therefore, we can directly execute show_ce without the judgment of > ie_modified() when lstat() has failed. > > Signed-off-by: ZheNing Hu <adlternative@gmail.com> > --- > builtin/ls-files.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) Looks good. I think we are finished with this part, except for one nit. > + if (stat_err && show_deleted) > show_ce(repo, dir, ce, fullname.buf, tag_removed); > - if (show_modified && ie_modified(repo->index, ce, &st, 0)) > - show_ce(repo, dir, ce, fullname.buf, tag_modified); > + if (show_modified && > + (stat_err || ie_modified(repo->index, ce, &st, 0))) > + show_ce(repo, dir, ce, fullname.buf, tag_modified); > } The last line is misindented by having one leading horizontal tab too many. show_ce() for modified files and show_ce() for deleted files are done independently under different conditions and stand as equals, so the beginning of them should align to show that. Perhaps format the last three lines more like so: if (show_modified && (stat_err || ie_modified(repo->index, ce, &st, 0))) show_ce(repo, dir, ce, fullname.buf, tag_modified); Again this would cascade throughout the sreies, but let's see if there are other things we may want to change in the rest of the series first. Otherwise, instead of having you rebase, I probably have time to tweak the series on my end while queuing. Thanks.
diff --git a/builtin/ls-files.c b/builtin/ls-files.c index c8eae899b82..1e264bd1329 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -335,7 +335,7 @@ static void show_files(struct repository *repo, struct dir_struct *dir) for (i = 0; i < repo->index->cache_nr; i++) { const struct cache_entry *ce = repo->index->cache[i]; struct stat st; - int err; + int stat_err; construct_fullname(&fullname, repo, ce); @@ -346,11 +346,14 @@ static void show_files(struct repository *repo, struct dir_struct *dir) continue; if (ce_skip_worktree(ce)) continue; - err = lstat(fullname.buf, &st); - if (show_deleted && err) + stat_err = lstat(fullname.buf, &st); + if (stat_err && (errno != ENOENT && errno != ENOTDIR)) + error_errno("cannot lstat '%s'", fullname.buf); + if (stat_err && show_deleted) show_ce(repo, dir, ce, fullname.buf, tag_removed); - if (show_modified && ie_modified(repo->index, ce, &st, 0)) - show_ce(repo, dir, ce, fullname.buf, tag_modified); + if (show_modified && + (stat_err || ie_modified(repo->index, ce, &st, 0))) + show_ce(repo, dir, ce, fullname.buf, tag_modified); } }