From: Patrick Steinhardt <ps@pks.im>
Date: Wed, 11 Mar 2026 14:01:06 GMT
Subject: Re: [PATCH 4/6] object-file: generalize counting objects
Message-ID: <abF1opEays8LQYbr@pks.im>
In-Reply-To: <87pl5albfz.fsf@iotcl.com>

On Wed, Mar 11, 2026 at 02:53:20PM +0100, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/object-file.c b/object-file.c
> > index da67e3c9ff..d35cec201f 100644
> > --- a/object-file.c
> > +++ b/object-file.c
> > @@ -1868,40 +1868,56 @@ int odb_source_loose_for_each_object(struct odb_source *source,
> >  					     NULL, NULL, &data);
> >  }
> >  
> > -int odb_source_loose_approximate_object_count(struct odb_source *source,
> > -					      unsigned long *out)
> > +static int count_loose_object(const struct object_id *oid UNUSED,
> > +			      struct object_info *oi UNUSED,
> > +			      void *payload)
> > +{
> > +	unsigned long *count = payload;
> > +	(*count)++;
> > +	return 0;
> > +}
> > +
> > +int odb_source_loose_count_objects(struct odb_source *source,
> > +				   enum odb_count_objects_flags flags,
> > +				   unsigned long *out)
> >  {
> >  	const unsigned hexsz = source->odb->repo->hash_algo->hexsz - 2;
> > -	unsigned long count = 0;
> > -	struct dirent *ent;
> >  	char *path = NULL;
> >  	DIR *dir = NULL;
> >  	int ret;
> >  
> > -	path = xstrfmt("%s/17", source->path);
> > +	if (flags & ODB_COUNT_OBJECTS_APPROXIMATE) {
> > +		unsigned long count = 0;
> > +		struct dirent *ent;
> >  
> > -	dir = opendir(path);
> > -	if (!dir) {
> > -		if (errno == ENOENT) {
> > -			*out = 0;
> > -			ret = 0;
> > +		path = xstrfmt("%s/17", source->path);
> > +
> > +		dir = opendir(path);
> > +		if (!dir) {
> > +			if (errno == ENOENT) {
> > +				*out = 0;
> > +				ret = 0;
> > +				goto out;
> > +			}
> > +
> > +			ret = error_errno("cannot open object shard '%s'", path);
> >  			goto out;
> >  		}
> >  
> > -		ret = error_errno("cannot open object shard '%s'", path);
> > -		goto out;
> > -	}
> > +		while ((ent = readdir(dir)) != NULL) {
> > +			if (strspn(ent->d_name, "0123456789abcdef") != hexsz ||
> > +			    ent->d_name[hexsz] != '\0')
> > +				continue;
> > +			count++;
> > +		}
> >  
> > -	while ((ent = readdir(dir)) != NULL) {
> > -		if (strspn(ent->d_name, "0123456789abcdef") != hexsz ||
> > -		    ent->d_name[hexsz] != '\0')
> > -			continue;
> > -		count++;
> > +		*out = count * 256;
> > +		ret = 0;
> > +	} else {
> > +		ret = odb_source_loose_for_each_object(source, NULL, count_loose_object,
> > +						       out, 0);
> 
> Isn't `*out` uninitialized here? Should we add `*out = 0;` before this
> line?

Oh, indeed. Will fix, thanks!

Patrick