git/list[1] front-page[2] threads[3] people[4] search[5] about
 

RE: [Fix v1] builtin/ls-files.c: add error check on lstat for modified files

From
Randall S. Becker <rsbecker@nexbridge.com>
Date
Feb 17, 2019, 17:29 UTC
Message-ID
<000001d4c6e6$52475010$f6d5f030$@nexbridge.com>
In-Reply-To
<e762e80a-d1ef-f906-e128-03f35de3c3ba@ramsayjones.plus.com>
On February 17, 2019 12:05, Ramsay Jones wrote:
Show 30 quoted lines
> On 17/02/2019 16:34, randall.s.becker@rogers.com wrote:
> > From: "Randall S. Becker" <rsbecker@nexbridge.com>
> >
> > The result from lstat, checking whether a file has been deleted, is
> > now included priot to calling id_modified when showing modified files.
> > Prior
> 
> s/priot/prior/; s/id_modified/ie_modified/
> 
> > to this fix, it is possible that files that were deleted could show up
> > as being modified because the lstat error was unchecked.
> >
> > Reported-by: Joe Ranieri <jranieri@grammatech.com>
> > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
> > ---
> >  builtin/ls-files.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/builtin/ls-files.c b/builtin/ls-files.c index
> > 29a8762d4..fc21f4795 100644
> > --- a/builtin/ls-files.c
> > +++ b/builtin/ls-files.c
> > @@ -348,7 +348,7 @@ static void show_files(struct repository *repo,
> struct dir_struct *dir)
> >  			err = lstat(fullname.buf, &st);
> >  			if (show_deleted && err)
> 
> To be pedantic, this should probably check for (err == ENOENT), since
> lstat() can fail for several reasons which don't imply that the path has been
> deleted. However, that is unlikely.
That would be very platform specific error checking. lstat can fail for a variety of other reasons also leaving the file deleted (like a symlink issue), but you are correct. I was following the prior line's model of checking for consistency.
Show 16 quoted lines
> No reason to include such a check in this patch, of course.
> 
> ATB,
> Ramsay Jones
> 
> >  				show_ce(repo, dir, ce, fullname.buf,
> tag_removed);
> > -			if (show_modified && ie_modified(repo->index, ce,
> &st, 0))
> > +			if (show_modified && !err && ie_modified(repo-
> >index, ce, &st, 0))
> >  				show_ce(repo, dir, ce, fullname.buf,
> tag_modified);
> >  		}
> >  	}
> >
This was just to address what Joe reported earlier. It seemed like an easy one to address.

Regards, Randall

Previous: Ramsay JonesNext: Junio C Hamano
Message 3 of 4 in “[Fix v1] builtin/ls-files.c: add error check on lstat for modified files”
  1. randall.s.becker@rogers.comFeb 17, 2019
  2. Ramsay JonesFeb 17, 2019
  3. Randall S. BeckerFeb 17, 2019
  4. Junio C HamanoApr 1, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.