Re: [PATCH v6 8/9] config: make `packed_git_(limit|window_size)` non-global variables
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 8, 2024, 03:42 UTC
- Message-ID
- <xmqqbjyqpf1u.fsf@gitster.g>
- In-Reply-To
- <3a8e3b88df28ed65cc8fbce3d51dd1cc19581e7e.1730976185.git.karthik.188@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
Show 8 quoted lines
> 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?
Show 7 quoted lines
> 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?
Show 20 quoted lines
> +
> + 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.