On Mon, Jun 21, 2021 at 08:32:56PM +0200, René Scharfe wrote:
> Am 20.06.21 um 22:03 schrieb SZEDER Gábor:
> > RFC!! Alas, not calling stop_progress() on error has drawbacks:
> >
> > - All memory allocated for the progress bar is leaked.
> > - This progress line remains "active", in the sense that if we were
> > to start a new progress later in the same git process, then with
> > GIT_TEST_CHECK_PROGRESS it would trigger the other BUG() catching
> > nested/overlapping progresses.
> >
> > Do we care?! TBH I don't :)
> > Anyway, if we do, then we might need some sort of an abort_progress()
> > function...
>
> I think the abort_progress() idea makes sense; to clean up allocations,
> tell the user what happened and avoid the BUG(). Showing just
> "aborted" instead of "done" should suffice here -- the explanation is
> given a few lines later ("'foo' was not filtered properly").
Very well put. I concur that having an abort_progress() API makes sense
for all of the reasons that you suggest, but also because we shouldn't
encourage not using what seems like an appropriate API in order to not
fail tests when GIT_TEST_CHECK_PROGRESS is set.
Thanks,
Taylor
