Paul Eggert wrote: > Thanks for all that work to make fts better! A couple of minor things > about comments: > > On 08/18/2011 06:53 AM, Jim Meyering wrote: > >> + into memory at once. However, When an fts_compar function > > The "However," can be removed (there are too many Buts etc. in the > neighborhood already ...). > >> + The other conditionals ensure >> + that we are using the *at functions (FTS_CWDFD) and that we >> + are not in no-chdir mode (induced by use of FTS_LOGICAL). */ > > Are these other conditionals independent of whether we want to avoid > putting too many entries in RAM? If so, perhaps we should remove these > other conditionals; if not, it'd help for the comment to explain why not.
Initially I didn't even write that "The other conditionals..." sentence. I agree that it doesn't say enough. The main motivation was to solve the problems that were most common, and to avoid getting bogged down in the less-common use cases. Already, this change is defending against something very unusual: applying common tools to directories with millions of entries. Worrying about applying "du -L" (FTS_LOGICAL) on such a directory is far lower priority. I'll add a FIXME saying that we should try to remove those conditionals, but that doing so will require careful testing of those less-common use cases. Actually, as I write this, I confess I'm thinking that the marginal gain is not worth the risk.