Am 18.01.22 um 18:25 schrieb Junio C Hamano: > René Scharfe <l.s.r@xxxxxx> writes: > >>> This series really feels like it's adding too much complexity and >>> potential auditing headaches (distributors worrying about us shipping a >>> CSPRNG, having to audit it) to a low-level codepath that most of the >>> time won't need this at all. >> >> Good point. Please let me think out loud for a moment. > > Yeah, I agree you and Ævar that the topic may be over-engineering > the solution for problem that we shouldn't be the ones who solve. > > I agree with your analysis that the "diff" tempfiles do need suffix, > we SHOULD create them in $TMPDIR (not in the working tree or > $GIT_DIR) to support operation in a read-only repository, but we can > create a unique temporary directory and place a file (even under its > original name) in it as a workaround. I forgot one crucial aspect, though: umask. The "m" variants of the tempfile functions set a mode, with umask applied. git_mkstemps_mode() does that by passing the mode to open(2), which applies the umask internally. Neither mkstemp(3) nor chmod(2) do that for us, so a replacement needs to call umask(2) to get it and again to restore it, which requires two system calls and is racy if multiple threads do this. Diff doesn't need a custom mode, so we can still use mkdtemp() there and thus make the suffix feature of git_mkstemps_mode() unnecessary. But a replacement for git_mkstemp_mode() with two umask(2) calls looks less attractive to me than fortifying git_mkstemps_mode() with a good source of randomness. René
