Re: [PATCH] fsck: print progress

[Jeff King <peff@peff.net>]

On Thu, Nov 03, 2011 at 10:21:53AM +0700, Nguyen Thai Ngoc Duy wrote:

> fsck is usually a long process and it would be nice if it prints
> progress from time to time.

The output looks good to me. Code looks sane overall, with one comment:

> +		for (i = 1, p = packed_git; p; p = p->next, i++) {
> +			if (show_progress) {
> +				char buf[32];
> +				snprintf(buf, sizeof(buf), "Verifying pack %d/%d",
> +					 i, nr_packs);
> +				if (open_pack_index(p))
> +					continue;
> +				progress = start_progress(buf, p->num_objects);
> +			}
>  			/* verify gives error messages itself */
> -			verify_pack(p);
> +			verify_pack(p, progress);
> +
> +			/*
> +			 * we do not stop progress here, let the next
> +			 * progress line overwrite the current one for
> +			 * the next pack.
> +			 */
> +		}
> +		stop_progress(&progress);

We're actually leaking some memory here, since stop_progress will also
free() the progress object and any associated resources. It's not a lot,
but it's kind of ugly.

Perhaps there should be a special version of stop_progress that handles
this better? Or perhaps we could even come up with a total object count
before starting.  I guess it would involve mapping each pack index
simultaneously, though by my reading of the code, I think we do that
anyway (the opened index is cached in the pack object).

-Peff