On 7/8/2020 10:10 PM, Matheus Tavares wrote:
> In 11179eb311 ("entry.c: check if file exists after checkout",
> 2017-10-05) we started checking the result of the lstat() call done
> after writing a file, to avoid writing garbage to the corresponding
> cache entry. However, the code skips calling lstat() if it's possible
> to use fstat() when it still has the file descriptor open. And when
> calling fstat() we don't do the same error checking. To fix that, let
> the callers of fstat_output() know when fstat() fails. In this case,
> write_entry() will try to use lstat() and properly report an error if
> that fails as well.
Looking at this for the first time, I was confused because 11179eb311
doesn't touch these lines. But that's the point: it should have.
Thanks for finding this! I wonder if there is a way to expose this
behavior in a test... it definitely seems like this is only something
that happens if there is a failure in the filesystem, so I'm not sure
such a thing is possible.
It would just be nice to know the ramifications of this change in
behavior, keeping in mind that this behavior started way back in
e4c7292353 (write_entry(): use fstat() instead of lstat() when file is
open, 2009-02-09), over 11 years ago!
Thanks,
-Stolee
> Signed-off-by: Matheus Tavares <matheus.bernardino@xxxxxx>
> ---
> entry.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/entry.c b/entry.c
> index 00b4903366..449bd32dee 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -113,8 +113,7 @@ static int fstat_output(int fd, const struct checkout *state, struct stat *st)
> /* use fstat() only when path == ce->name */
> if (fstat_is_reliable() &&
> state->refresh_cache && !state->base_dir_len) {
> - fstat(fd, st);
> - return 1;
> + return !fstat(fd, st);
> }
> return 0;
> }
>
