On Tue, Mar 01 2022, Jeff Hostetler via GitGitGadget wrote:
> From: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
I know this is in "next", just looking over this code again...
> +static void lookup_fsmonitor_settings(struct repository *r)
Here we'll start loading the settings...
> +{
> + struct fsmonitor_settings *s;
> + const char *const_str;
> + int bool_value;
> +
> + if (r->settings.fsmonitor)
> + return;
MARK
> + CALLOC_ARRAY(s, 1);
> +
> + r->settings.fsmonitor = s;
And right after we alloc the r->settings.fsmonitor we'll ...
> + fsm_settings__set_disabled(r);
...call this method...
> +
> + /*
> + * Overload the existing "core.fsmonitor" config setting (which
> + * has historically been either unset or a hook pathname) to
> + * now allow a boolean value to enable the builtin FSMonitor
> + * or to turn everything off. (This does imply that you can't
> + * use a hook script named "true" or "false", but that's OK.)
> + */
> + switch (repo_config_get_maybe_bool(r, "core.fsmonitor", &bool_value)) {
> +
> + case 0: /* config value was set to <bool> */
> + if (bool_value)
> + fsm_settings__set_ipc(r);
> + return;
> +
> + case 1: /* config value was unset */
> + const_str = getenv("GIT_TEST_FSMONITOR");
> + break;
> +
> + case -1: /* config value set to an arbitrary string */
> + if (repo_config_get_pathname(r, "core.fsmonitor", &const_str))
> + return; /* should not happen */
> + break;
> +
> + default: /* should not happen */
> + return;
> + }
> +
> + if (!const_str || !*const_str)
> + return;
> +
> + fsm_settings__set_hook(r, const_str);
> +}
> [...]
> +void fsm_settings__set_disabled(struct repository *r)
> +{
> + if (!r)
> + r = the_repository;
> +
> + lookup_fsmonitor_settings(r);
...which here will recurse into lookup_fsmonitor_settings and hit
"MARK".
So isn't that fsm_settings__set_disabled() within that method pointless?
> + r->settings.fsmonitor->mode = FSMONITOR_MODE_DISABLED;
> + FREE_AND_NULL(r->settings.fsmonitor->hook_path);
It seems as though the intent was to reach this, but these all happen to
be the same thing you'd get with CALLOC_ARRAY(), so I think this just
happened to work out...
> +enum fsmonitor_mode {
> + FSMONITOR_MODE_DISABLED = 0,
...I.e. this is luckily zero.
