Re: [PATCH 1/7] bundle-uri: create bundle_list struct and helpers
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 22, 2022, 17:57 UTC
- Message-ID
- <xmqqfshoduub.fsf@gitster.g>
- In-Reply-To
- <c39438886586f30b020e8ac473c7facaa757f822.1661181174.git.gitgitgadget@gmail.com>
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 29 quoted lines
> +/**
> + * The remote_bundle_info struct contains information for a single bundle
> + * URI. This may be initialized simply by a given URI or might have
> + * additional metadata associated with it if the bundle was advertised by
> + * a bundle list.
> + */
> +struct remote_bundle_info {
> + struct hashmap_entry ent;
> +
> + /**
> + * The 'id' is a name given to the bundle for reference
> + * by other bundle infos.
> + */
> + char *id;
> +
> + /**
> + * The 'uri' is the location of the remote bundle so
> + * it can be downloaded on-demand. This will be NULL
> + * if there was no table of contents.
> + */
> + char *uri;
> +
> + /**
> + * If the bundle has been downloaded, then 'file' is a
> + * filename storing its contents. Otherwise, 'file' is
> + * an empty string.
> + */
> + struct strbuf file;
> +};Presumably the sequence of events are that first a bundle list is obtained, with their .file member set to empty, then http worker(s) download and deposit the contents to files at which time the .file member is set to the resulting file. The file downloader presumably uses the usual "create a temporary file, download to it, and then commit it by closing and then renaming" dance, and the downloading http worker may want to have two strbufs somewhere it can access to come up with the name of the temporary and the name of the final file. But once the result becomes a committed file, its name will not change, or will it?
At this step without the code that actually uses the data, use of strbuf, instead of "char *" like id and uri members do, smells like a premature optimization, and it is unclear if the optimization is even effective.
Other than that, looks good to me.