On Mon, Jun 13, 2022 at 04:20:57PM +0200, Omar Polo wrote:
> > +	/*
> > +	 * Find the largest pack which contains at least some of the
> > +	 * commits and tags we are interested in.
> > +	 */
> > +	TAILQ_FOREACH(pe, &repo->packidx_paths, entry) {
> > +		const char *path_packidx = pe->path;
> > +		struct got_packidx *packidx;
> > +		int nobj, i, idx, ncommits = 0;
> > +
> > +		err = got_repo_get_packidx(&packidx, path_packidx, repo);
> 
> assuming this fails
> 
> > +		if (err)
> > +			break;
> > +
> > +		nobj = be32toh(packidx->hdr.fanout_table[0xff]);
> > +		if (nobj <= nobj_max)
> > +			continue;
> > +
> > +		for (i = 0; i < nids; i++) {
> > +			idx = got_packidx_get_object_idx(packidx, ids[i]);
> > +			if (idx != -1)
> > +				ncommits++;
> > +		}
> > +		if (ncommits > ncommits_max) {
> > +			best_packidx_path = path_packidx;
> > +			nobj_max = nobj;
> > +			ncommits_max = ncommits;
> > +		}
> > +	}
> 
> shouldn't we skip getting the path here?  (either turning the break into
> a return or checking err here.)

Indeed. The if-statement below should include: && err == NULL
I'll fix this up before commit. Thanks!

 
> > +	if (best_packidx_path) {
> > +		err = got_repo_get_packidx(best_packidx, best_packidx_path,
> > +		    repo);
> > +	}
> > +
> > +	return err;