Re: [PATCH 2/7] builtin/gc: refactor to read config into structure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/13/24 3:17 AM, Patrick Steinhardt wrote:
The git-gc(1) command knows to read a bunch of config keys to tweak its
own behaviour. The values are parsed into global variables, which makes
it hard to correctly manage the lifecycle of values that may require a
memory allocation.

Refactor the code to use a `struct gc_config` that gets populated and
passed around. For one, this makes previously-implicit dependencies on
these config values clear. Second, it will allow us to properly manage
the lifecycle in the next commit.

I think this is a valuable goal.

-static const char *gc_log_expire = "1.day.ago";
-static const char *prune_expire = "2.weeks.ago";
-static const char *prune_worktrees_expire = "3.months.ago";

I was going to mention this in the previous patch where you change how
these variables are cast into git_config_get_expiry(). They aren't
changing to non-const here, either.

+struct gc_config {
...
+	const char *gc_log_expire;
+	const char *prune_expire;
+	const char *prune_worktrees_expire;

The fact that they are initialized to const strings makes it
difficult to know if they've been updated. I wonder if we need
to change them to have a "if NULL, then use a const default"
somewhere. (And maybe you do this later in the series).

+static void gc_config(struct gc_config *cfg)

I appreciate that you are taking the step to make the structure
a process parameter and not just another global.

Thanks,
-Stolee





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux