> On 4/11/26 06:55, LorenzoPegorari wrote:
> 
> > @@ -171,19 +172,15 @@ static void finish_repacking_promisor_objects(struct repository *repo,
> >   		/*
> >   		 * pack-objects creates the .pack and .idx files, but not the
> > -		 * .promisor file. Create the .promisor file, which is empty.
> > -		 *
> > -		 * NEEDSWORK: fetch-pack sometimes generates non-empty
> > -		 * .promisor files containing the ref names and associated
> > -		 * hashes at the point of generation of the corresponding
> > -		 * packfile, but this would not preserve their contents. Maybe
> > -		 * concatenate the contents of all .promisor files instead of
> > -		 * just creating a new empty file.
> > +		 * .promisor file. Create the .promisor file.
> >   		 */
> >   		promisor_name = mkpathdup("%s-%s.promisor", packtmp,
> >   					  line.buf);
> >   		write_promisor_file(promisor_name, NULL, 0);
> > +		/* Now let's fill the content of the newly created .promisor file */
> > +		copy_promisor_content(repo, line.buf, packtmp, not_repacked_basenames);
> 
> Here, the file opened by copy_promisor_content() is an empty file. Is this
> line necessary? ;)
> 
> ...hold on. I recall you mentioning in one of the versions that you had
> downgraded this helper from a generic function to a static one. Since it now
> only serves this particular business logic, I think the implementation
> should be tweaked slightly as well.

> > +	/* Open the .promisor dest file, and fill dest_content with its content */
> > +	dest_promisor_name = mkpathdup("%s-%s.promisor", packtmp, dest_hex);
> > +	dest = xfopen(dest_promisor_name, "r+");
> > +	while (strbuf_getline(&line, dest) != EOF)
> > +		strset_add(&dest_content, line.buf);
> 
> If file contains a large number of unique lines, dest_to_write, which is a
> strbuf, may keep realloc memory until the loop ends, at which point all the
> memory is released. I wonder if this might be wasting some heap.
> 
> If it were me, I might write it like this:
> 
> 	struct strset seen_lines = STRSET_INIT;
> 	dest = xfopen(dest_promisor_name, "w");
> 	while (strbuf_getline(&line, source) != EOF) {
>     		if (strset_add(&seen_lines, line.buf)) {
>         		fprintf(dest, "%s\n", line.buf);
>     		}	
> 	}
> 
> It also prevents file pointer misalignment.