On Thu, Mar 29, 2018 at 05:18:30PM +0200, Johannes Schindelin wrote: > Little did I know that this would turn not only into a full patch to fix this > issue, but into a full-blown series of nine patches. It's amazing how often that happens. :) > The first patch is somewhat of a "while at it" bug fix that I first thought > would be a lot more critical than it actually is: It really only affects config > files that start with a section followed immediately (i.e. without a newline) > by a one-letter boolean setting (i.e. without a `= <value>` part). So while it > is a real bug fix, I doubt anybody ever got bitten by it. That makes me wonder if somebody could craft a malicious config to do something bad. But I don't think so. Config is trusted already, and it looks like this bug is both hard to trigger and doesn't result in any kind of memory funniness, just a bogus output. > Now, to the really important part: why does this patch series not conflict with > my very early statements that we cannot simply remove empty sections because we > may end up with stale comments? > > Well, the patch in question takes pains to determine *iff* there are any > comments surrounding, or included in, the section. If any are found: previous > behavior. Under the assumption that the user edited the file, we keep it as > intact as possible (see below for some argument against this). If no comments > are found, and let's face it, this is probably *the* common case, as few people > edit their config files by hand these days (neither should they because it is > too easy to end up with an unparseable one), the now-empty section *is* > removed. I'm not against people editing their config files by hand. But I think what you propose here makes a lot of sense, because it works as long as you don't intermingle hand- and auto-editing in the same section (and it even works if you do intermingle, as long as you don't use comments, which are probably even more rare). So it seems like quite a sensible compromise, and I think should make most people happy. -Peff
