Re: [PATCH 06/34] clone: release strbuf after use in remove_junk()
- From
Jeff King <peff@peff.net>
- Date
- Sep 13, 2017, 12:56 UTC
- Message-ID
- <20170913125611.zdy3r5gijz5ohl2r@sigill.intra.peff.net>
- In-Reply-To
- <1c0918c7-8630-e545-6c34-ab03995797bb@web.de>
On Mon, Sep 11, 2017 at 11:40:05PM +0200, René Scharfe wrote:
Show 8 quoted lines
> > Yes, but I didn't want to touch each code site that creates a file, > > which is a lot more invasive. I expect expanding to 4096 (or PATH_MAX) > > would be sufficient in practice. > > Perhaps it is in most cases. Having certainty would be better. We can > leave unpack_trees() untouched and instead traverse the tree beforehand, > in order to find the longest path. Perhaps we can do something similar > for init_db().
I wonder if it would be possible to just wrap open() in a transparent way.
Show 8 quoted lines
> > I'd also note that the current code isn't _remotely_ async safe and > > nobody's complained. So I'm perfectly happy doing nothing, too. I care > > a bit more about the tempfile.c interface because it's invoked a lot > > more frequently. > > I guess clones are not interrupted very often during checkout; same > with worktree creation. So perhaps nasty things could happen with a > low probability, but nobody tried hard enough to hit that case?
Right, that's my guess. And "nasty" is quite likely to be "deadlocks on a malloc or stdio lock". Which is not the end of the world.
Show 12 quoted lines
> > No, I think that's how you'd have to do it. The implementation isn't all > > that bad, but hitting every file-creation would be invasive. I'd > > almost rather bail to exec-ing "rm -rf $dir", but we probably can't do > > _that_ in a signal-handler either. :) > > Well, fork(2), execve(2), and waitpid(2) are on the list, so actually > you can. > > mingw_spawnvpe(), which is used by start_command() on Windows, > allocates memory, though. Preparing environment and argument list > in advance and just calling CreateProcess() in the signal handler > might work.
Yeah, I imagine it's do-able with enough advance effort.
Given the lack of reports and the rapidly expanding complexity, I'm not planning on looking further into this for now.
-Peff