Re: [PATCH 05/11] last-modified: handle repo_parse_commit() failures

[Junio C Hamano <gitster@pobox.com>]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> Skip unparsable commits by checking the return value and
> continuing to the next iteration (or returning early in
> process_parent). This matches the defensive pattern used in other
> revision walkers such as limit_list() and get_revision_internal().
> ...
> @@ -414,12 +415,14 @@ static int last_modified_run(struct last_modified *lm)
>  		 * Otherwise, make sure that 'c' isn't reachable from anything
>  		 * in the '--not' queue.
>  		 */
> -		repo_parse_commit(lm->rev.repo, c);
> +		if (repo_parse_commit(lm->rev.repo, c))
> +			continue;

Shouldn't this be

			goto cleanup;

instead?  'n' pulled out of not_queue may be unparseable and when we
ignore it, don't we still want to clean up the active_paths slab for
commit 'c'?

>  		while ((n = prio_queue_get(&not_queue))) {
>  			struct commit_list *np;
>  
> -			repo_parse_commit(lm->rev.repo, n);
> +			if (repo_parse_commit(lm->rev.repo, n))
> +				continue;
>  
>  			for (np = n->parents; np; np = np->next) {
>  				if (!(np->item->object.flags & PARENT2)) {