From: Patrick Steinhardt Date: Wed, 12 Nov 2025 10:21:08 GMT Subject: Re: [PATCH] attr: avoid recursion when expanding attribute macros Message-ID: In-Reply-To: <20251112071651.GB431661@coredump.intra.peff.net> On Wed, Nov 12, 2025 at 02:16:51AM -0500, Jeff King wrote: > On Wed, Nov 12, 2025 at 07:57:14AM +0100, Patrick Steinhardt wrote: > > > So personally I would've probably leaned into the direction of enforcing > > a hard limit. I don't see a reason why anybody would need more than a > > couple of recursions, it culls both compute and memory growth, and it > > allows us to have a proper error message in case the limit is busted. > > Furthermore, we can demonstrate right now that it wasn't possible to > > have unlimited recursion anyway, which makes it easier to put a new > > limit into place. > > > > But following my above reasoning I think it's okay to turn this into > > iteration, as well, though, but I'd like to hear whether my train of > > thought matches yours. > > Yeah, it does match mine. If I wanted to waste a bunch of CPU and memory > on a hosting site, there are a lot easier ways to do that than with > really long gitattributes. > > I'm not at all opposed to putting in a hard limit on top. My general > feeling is that it never hurts to convert recursion to iteration; it > only gives us more options. I'm not planning to work on a hard limit > myself, but if you want to, be my guest. :) > > I think if we do (or even if we don't), it may also be reasonable to > shrink the max attribute file size to 10MB or even smaller. I think for now it's okay to convert this into iteration and not introduce a limit, at least as long as we keep an open mind about introducing such a limit in the future. I don't really expect that anyone will ever abuse this, but if I'm wrong and this happens at one point in time we may have to introduce the limit retroactively. So: I'm happy with your patch, but it might make sense to summarize the discussion in the commit message. Thanks! Patrick