Re: [PATCH] fsck: print progress

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