On 7/21/2020 4:00 PM, Junio C Hamano wrote: > Matheus Tavares Bernardino <matheus.bernardino@xxxxxx> writes: > >> I was looking further at this code and noticed that the conditions >> under which we fstat() (or lstat()) an entry are slightly different >> throughout entry.c: >> >> - In write_entry()'s footer, we call lstat() iff stat->refresh_cache. >> - In write_entry()'s `write_file_entry` label, we call fstat_output() >> when !to_tempfile. >> - In streaming_write_entry() we call fstat_output() without checking >> if !to_tempfile. >> - And, finally, in fstat_output() itself, we check >> `state->refresh_cache && !state->base_dir_len`. >> >> I understand we always check state->refresh_cache to avoid getting >> stat information we won't really need later, as we are not updating >> the index. But why do we check !to_tempfile and !state->base_dir_len? >> Doesn't writing to a tempfile or using a checkout prefix already imply >> !state->refresh_cache? > > You can easily blame the code back to e4c72923 (write_entry(): use > fstat() instead of lstat() when file is open, 2009-02-09). Back > then, only a single place assigned 0 to state.refresh_cache and that > is in "checkout-index" with either base_dir_len or to_tempfile set. > > I do not remember, and I am fairly sure Stolee does not remember > either. If I have to guess, this was done merely to be extra > cautious, perhaps? As refresh_cache bit is checked first, check for > !to_tempfile and !base_dir_len would be dead at best and redundant > at worst. Yeah, this portion is way outside of my expertise. I'm happy to _try_ reading patches, but I'd have difficulty being confident in any change in this area. Thanks, -Stolee
