Tao Klerks <tao@xxxxxxxxxx> writes: >> > + configured_dir_flags = configured_default_dir_flags(istate); >> > + if (dir->flags != configured_dir_flags) >> > + return NULL; >> >> Hmph. If this weren't necessary, this function does not need to >> call configured_default_dir_flags(), and it can lose the >> configured_dir_flags variable, too. Which means that >> new_untracked_cache() function does not need to take the flags word >> as a caller-supplied parameter. Instead, it can make a call to >> configured_dir_flags() and assign the result to uc->dir_flags >> itself, which would have been much nicer. > > I've tightened this up a little with an inline call to > configured_default_dir_flags(), getting rid of the variable, let's see > if that makes more sense / is cleaner. The extra variable does not bother me at all. Leaving the room for the caller to screw up and pass an incorrect configred_dir_flags is what disturbs me. If this caller needs to call configred_default_dir_flags(istate), then we cannot avoid it. And the extra variable needed to call the function only once, store its result, and use that result twice, is perfectly a good thing to have. We don't want to see inline or anything tricky. Thanks.
