On Mon, Jun 21, 2021 at 05:16:11PM +0200, Ævar Arnfjörð Bjarmason wrote: > This v2 addresses an omission noted by Andrei Rybak[1]. I didn't > factor out the "name" into a variable in 2/3 for ease of reading > 3/3. That's now done. This all looks OK to me. I left a few small comments on the patches themselves. The UNLEAK() thing I suggested for patch 1 does make that patch much smaller and easier to read, but I suspect makes patch 3 harder (i.e., you are reusing the "cleanup" sections there to do the bundle header release. That could _also_ get UNLEAKed, but at some point it becomes more clear to actually clean up after ourselves, and I think patch 3 probably crosses that point). So I'm OK to ignore that. I would prefer to see the "die()" thing I mentioned there addressed, as well as the ternary thing from patch 3. But neither of them is incorrect as-is; it's just a style/preference thing. -Peff
