Junio C Hamano <gitster@xxxxxxxxx> writes:
> Karthik Nayak <karthik.188@xxxxxxxxx> writes:
>
>> The variables `packed_git_window_size` and `packed_git_limit` are global
>> config variables used in the `packfile.c` file. Since it is only used in
>> this file, let's change it from being a global config variable to a
>> local variable for the subsystem.
>>
>> We do this by introducing a new local `packfile_config` struct in
>> `packfile.c` and also adding the required function to parse the said
>> config. We then use this within `packfile.c` to obtain the variables.
>
> This patch has no string "packfile_config" in it, other than the one
> in the above string. A stale description?
>
Yup indeed.
>> if (!win) {
>> - size_t window_align = packed_git_window_size / 2;
>> + size_t window_align;
>> off_t len;
>> + struct repo_settings *settings;
>> +
>> + /* lazy load the settings incase it hasn't been setup */
>
> "incase" -> "in case"?
>
Will change.
>> + prepare_repo_settings(p->repo);
>> + settings = &p->repo->settings;
>
> This change is curious. How can p->repo be uninitialized? p is a
> packed-git list created in some repository, surely it should already
> be initialized, no?
>
Here `p->repo` itself is expected to be initialized. We're however
trying to initialize `p->repo->settings`. Which might not have been. If
it is, `prepare_repo_settings` will return early.
>
>> +
>> + window_align = settings->packed_git_window_size / 2;
>> if (p->pack_fd == -1 && open_packed_git(p))
>> die("packfile %s cannot be accessed", p->pack_name);
>> @@ -661,11 +667,12 @@ unsigned char *use_pack(struct packed_git *p,
>> CALLOC_ARRAY(win, 1);
>> win->offset = (offset / window_align) * window_align;
>> len = p->pack_size - win->offset;
>> - if (len > packed_git_window_size)
>> - len = packed_git_window_size;
>> + if (len > settings->packed_git_window_size)
>> + len = settings->packed_git_window_size;
>> win->len = (size_t)len;
>> pack_mapped += win->len;
>> - while (packed_git_limit < pack_mapped
>> +
>> + while (settings->packed_git_limit < pack_mapped
>> && unuse_one_window(p))
>> ; /* nothing */
>> win->base = xmmap_gently(NULL, win->len,
>
> Other than that, the changes to the above block that uses the local
> variable "settings" looks good.
>
> Thanks.
Attachment:
signature.asc
Description: PGP signature
