Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: >> Is it defensive or is it hiding a problematic index under the rug? > > I wrote this defensive code only out of habit, not because I saw a > `ce_mode` that was 0. > >> If there is an index entry whose ce_mode is 0, I suspect we would >> want to error out with a BUG(), unless it is an intent-to-add entry. >> >> Shouldn't it cause an error to apply a patch that mucks with >> "newfile" after you did >> >> $ git add -N newfile >> >> If we allow ce_mode==0 to be propagated to st_mode, I suspect we >> will catch such a case with the "mode is different" warning code, at >> least. > > Is `ce_mode == 0` an indicator of a new file? In my tests, `git add -N` > will add the file with a non-zero mode... Oh, if we know nobody would assign 0 to ce_mode in a valid in-index entry, then we should (1) check and BUG() if we care there may be such a case due to a bug, or (2) assume that it never happens and omit the extra check. The third way in the patch is neither and is sweeping a potential bug ("potential" because the code apparently assumes it can happen) under the rug ("sweeping" because the code silently ignores such an abnormal case), I am afraid.