On Thu, Oct 21 2021, Emily Shaffer wrote: > On Thu, Oct 14, 2021 at 12:28:16AM +0200, Ævar Arnfjörð Bjarmason wrote: >> >> This series fixes various issues in and related to progress.c, and >> adds a BUG() assertion for us not starting two progress bars at the >> same time. Those changes are needed for subsequent changes that do >> more interesting things with this new global progress bar. >> >> This v3 hopefully addresses all the feedback on the v2, thanks >> all. Changes: >> >> * Fix a memory leak in 1/10, and make the progress tests use the >> SANITIZE=leak test mode. >> >> * Simplified some of the test-progress.c code (no more "start" >> handling, the "total" count is mandatory now. >> >> * Split out a formatting change into 2/10 to make 3/10 easier to >> read. >> >> * A new 9/10 makes an ad-hoc test recipie in 10/10 easier to explain >> (in response to Emily's comment). >> >> * The BUG() assertion in 10/10 now has a much better message, we dump >> the title of the two progress bars in play if we have a bug where >> we started two at the same time. > > One thing I noticed (and so did SZEDER) was the addition of string_list > to the helper; I made a comment accordingly in the patch where it was > added. It kind of seemed like an oversight - but I remember you were > wanting to head in that direction (multiple progress bar support) so > maybe it was paving the way for that? > > Anyways, with or without it the series looks OK to me in this round too. Replied to in the thread with SZEDER at: https://lore.kernel.org/git/211022.864k99kune.gmgdl@xxxxxxxxxxxxxxxxxxx/ I.e. it's not for multi-progress-bar support, but because the API doesn't xstrdup() the title. The loop handing it the title needs to keep the string it gave to it alive.
