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?
> 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"?
> + 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?
> +
> + 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.
