From 00df20a7ab4a4de355f69157acfd2502ab58a3aa Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 18 Sep 2023 18:29:40 -0400 Subject: [PATCH 1/8] fsmonitor: prefer repo_git_path() to git_pathdup() The fsmonitor_ipc__get_path() function ignores its repository argument. It should use it when constructing repo paths (though in practice, it is unlikely anything but the_repository is ever passed, so this is cleanup and future proofing, not a bug fix). Note that despite the lack of "dup" in the name, repo_git_path() behaves like git_pathdup() and returns an allocated string. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- compat/fsmonitor/fsm-ipc-win32.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compat/fsmonitor/fsm-ipc-win32.c b/compat/fsmonitor/fsm-ipc-win32.c index 8928fa93ce2239..41984ea48e26b8 100644 --- a/compat/fsmonitor/fsm-ipc-win32.c +++ b/compat/fsmonitor/fsm-ipc-win32.c @@ -6,6 +6,6 @@ const char *fsmonitor_ipc__get_path(struct repository *r) { static char *ret; if (!ret) - ret = git_pathdup("fsmonitor--daemon.ipc"); + ret = repo_git_path(r, "fsmonitor--daemon.ipc"); return ret; } From 42e862c0b3127736040790a11a4f44a0dee69888 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 18 Sep 2023 18:30:01 -0400 Subject: [PATCH 2/8] fsmonitor/win32: drop unused parameters A few helper functions (centered around file-watch events) take extra fsmonitor state parameters that they don't use. These are static helpers local to the win32 implementation, and don't need to conform to any particular interface. We can just drop the extra parameters, which simplifies the code and silences -Wunused-parameter. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- compat/fsmonitor/fsm-listen-win32.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/compat/fsmonitor/fsm-listen-win32.c b/compat/fsmonitor/fsm-listen-win32.c index a361a7db20edbf..90a2412284414e 100644 --- a/compat/fsmonitor/fsm-listen-win32.c +++ b/compat/fsmonitor/fsm-listen-win32.c @@ -289,8 +289,7 @@ void fsm_listen__stop_async(struct fsmonitor_daemon_state *state) SetEvent(state->listen_data->hListener[LISTENER_SHUTDOWN]); } -static struct one_watch *create_watch(struct fsmonitor_daemon_state *state, - const char *path) +static struct one_watch *create_watch(const char *path) { struct one_watch *watch = NULL; DWORD desired_access = FILE_LIST_DIRECTORY; @@ -361,8 +360,7 @@ static void destroy_watch(struct one_watch *watch) free(watch); } -static int start_rdcw_watch(struct fsm_listen_data *data, - struct one_watch *watch) +static int start_rdcw_watch(struct one_watch *watch) { DWORD dwNotifyFilter = FILE_NOTIFY_CHANGE_FILE_NAME | @@ -735,11 +733,11 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) state->listen_error_code = 0; - if (start_rdcw_watch(data, data->watch_worktree) == -1) + if (start_rdcw_watch(data->watch_worktree) == -1) goto force_error_stop; if (data->watch_gitdir && - start_rdcw_watch(data, data->watch_gitdir) == -1) + start_rdcw_watch(data->watch_gitdir) == -1) goto force_error_stop; for (;;) { @@ -755,7 +753,7 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) } if (result == -2) { /* retryable error */ - if (start_rdcw_watch(data, data->watch_worktree) == -1) + if (start_rdcw_watch(data->watch_worktree) == -1) goto force_error_stop; continue; } @@ -763,7 +761,7 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) /* have data */ if (process_worktree_events(state) == LISTENER_SHUTDOWN) goto force_shutdown; - if (start_rdcw_watch(data, data->watch_worktree) == -1) + if (start_rdcw_watch(data->watch_worktree) == -1) goto force_error_stop; continue; } @@ -776,7 +774,7 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) } if (result == -2) { /* retryable error */ - if (start_rdcw_watch(data, data->watch_gitdir) == -1) + if (start_rdcw_watch(data->watch_gitdir) == -1) goto force_error_stop; continue; } @@ -784,7 +782,7 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) /* have data */ if (process_gitdir_events(state) == LISTENER_SHUTDOWN) goto force_shutdown; - if (start_rdcw_watch(data, data->watch_gitdir) == -1) + if (start_rdcw_watch(data->watch_gitdir) == -1) goto force_error_stop; continue; } @@ -821,16 +819,14 @@ int fsm_listen__ctor(struct fsmonitor_daemon_state *state) data->hEventShutdown = CreateEvent(NULL, TRUE, FALSE, NULL); - data->watch_worktree = create_watch(state, - state->path_worktree_watch.buf); + data->watch_worktree = create_watch(state->path_worktree_watch.buf); if (!data->watch_worktree) goto failed; check_for_shortnames(data->watch_worktree); if (state->nr_paths_watching > 1) { - data->watch_gitdir = create_watch(state, - state->path_gitdir_watch.buf); + data->watch_gitdir = create_watch(state->path_gitdir_watch.buf); if (!data->watch_gitdir) goto failed; } From f4c5778b2dca447ae130cfab6cc9b7b614e7ec43 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 18 Sep 2023 18:31:48 -0400 Subject: [PATCH 3/8] fsmonitor: mark some maybe-unused parameters There's a bit of conditionally-compiled code in fsmonitor, so some function parameters may be unused depending on the build options: - in fsmonitor--daemon.c's try_to_run_foreground_daemon(), we take a detach_console argument, but it's only used on Windows. This seems intentional (and not mistakenly missing other platforms) based on the discussion in c284e27ba7 (fsmonitor--daemon: implement 'start' command, 2022-03-25), which introduced it. - in fsmonitor-setting.c's check_for_incompatible(), we pass the "ipc" flag down to the system-specific fsm_os__incompatible() helper. But we can only do so if our platform has such a helper. In both cases we can mark the argument as MAYBE_UNUSED. That annotates it enough to suppress the compiler's -Wunused-parameter warning, but without making it impossible to use the variable, as a regular UNUSED annotation would. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/fsmonitor--daemon.c | 2 +- fsmonitor-settings.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index 7e99c4d61ba959..7c4130c9811bd5 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -1412,7 +1412,7 @@ static int fsmonitor_run_daemon(void) return err; } -static int try_to_run_foreground_daemon(int detach_console) +static int try_to_run_foreground_daemon(int detach_console MAYBE_UNUSED) { /* * Technically, we don't need to probe for an existing daemon diff --git a/fsmonitor-settings.c b/fsmonitor-settings.c index b62acf44aee2b9..a6a9e6bc199ec2 100644 --- a/fsmonitor-settings.c +++ b/fsmonitor-settings.c @@ -62,7 +62,8 @@ static enum fsmonitor_reason check_remote(struct repository *r) } #endif -static enum fsmonitor_reason check_for_incompatible(struct repository *r, int ipc) +static enum fsmonitor_reason check_for_incompatible(struct repository *r, + int ipc MAYBE_UNUSED) { if (!r->worktree) { /* From caf433bbdf5516d6b17420c952795e8debdb2a6f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 18 Sep 2023 18:32:05 -0400 Subject: [PATCH 4/8] fsmonitor/win32: mark unused parameter in fsm_os__incompatible() We never look at the "ipc" argument we receive. It was added in 8f44976882 (fsmonitor: avoid socket location check if using hook, 2022-10-04) to support the darwin fsmonitor code. The win32 code has to match the same interface, but we should use an annotation to silence -Wunused-parameter. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- compat/fsmonitor/fsm-settings-win32.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compat/fsmonitor/fsm-settings-win32.c b/compat/fsmonitor/fsm-settings-win32.c index b6f67444944b5f..0f2aa321f6e15f 100644 --- a/compat/fsmonitor/fsm-settings-win32.c +++ b/compat/fsmonitor/fsm-settings-win32.c @@ -25,7 +25,7 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r) return FSMONITOR_REASON_OK; } -enum fsmonitor_reason fsm_os__incompatible(struct repository *r, int ipc) +enum fsmonitor_reason fsm_os__incompatible(struct repository *r, int ipc UNUSED) { enum fsmonitor_reason reason; From 4cb5e0b3b98c70912756551238b91fccf5f09c1e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 18 Sep 2023 18:32:38 -0400 Subject: [PATCH 5/8] fsmonitor: mark unused parameters in stub functions The fsmonitor code has some platform-specific functions for which one or more platforms implement noop or stub functions. We can't get rid of these functions nor change their interface, since the point is to match their equivalents in other platforms. But let's annotate their parameters to quiet the compiler's -Wunused-parameter warning. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- compat/fsmonitor/fsm-health-darwin.c | 8 ++++---- compat/fsmonitor/fsm-path-utils-win32.c | 7 ++++--- fsmonitor-ipc.c | 10 +++++----- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/compat/fsmonitor/fsm-health-darwin.c b/compat/fsmonitor/fsm-health-darwin.c index 5b1709d63f729b..c2afcbe6c89d9a 100644 --- a/compat/fsmonitor/fsm-health-darwin.c +++ b/compat/fsmonitor/fsm-health-darwin.c @@ -4,21 +4,21 @@ #include "fsm-health.h" #include "fsmonitor--daemon.h" -int fsm_health__ctor(struct fsmonitor_daemon_state *state) +int fsm_health__ctor(struct fsmonitor_daemon_state *state UNUSED) { return 0; } -void fsm_health__dtor(struct fsmonitor_daemon_state *state) +void fsm_health__dtor(struct fsmonitor_daemon_state *state UNUSED) { return; } -void fsm_health__loop(struct fsmonitor_daemon_state *state) +void fsm_health__loop(struct fsmonitor_daemon_state *state UNUSED) { return; } -void fsm_health__stop_async(struct fsmonitor_daemon_state *state) +void fsm_health__stop_async(struct fsmonitor_daemon_state *state UNUSED) { } diff --git a/compat/fsmonitor/fsm-path-utils-win32.c b/compat/fsmonitor/fsm-path-utils-win32.c index c8a3e9dcdbb655..f4f9cc1f336720 100644 --- a/compat/fsmonitor/fsm-path-utils-win32.c +++ b/compat/fsmonitor/fsm-path-utils-win32.c @@ -132,7 +132,8 @@ int fsmonitor__is_fs_remote(const char *path) /* * No-op for now. */ -int fsmonitor__get_alias(const char *path, struct alias_info *info) +int fsmonitor__get_alias(const char *path UNUSED, + struct alias_info *info UNUSED) { return 0; } @@ -140,8 +141,8 @@ int fsmonitor__get_alias(const char *path, struct alias_info *info) /* * No-op for now. */ -char *fsmonitor__resolve_alias(const char *path, - const struct alias_info *info) +char *fsmonitor__resolve_alias(const char *path UNUSED, + const struct alias_info *info UNUSED) { return NULL; } diff --git a/fsmonitor-ipc.c b/fsmonitor-ipc.c index 88575aa54cad07..153918cf768c48 100644 --- a/fsmonitor-ipc.c +++ b/fsmonitor-ipc.c @@ -20,7 +20,7 @@ int fsmonitor_ipc__is_supported(void) return 0; } -const char *fsmonitor_ipc__get_path(struct repository *r) +const char *fsmonitor_ipc__get_path(struct repository *r UNUSED) { return NULL; } @@ -30,14 +30,14 @@ enum ipc_active_state fsmonitor_ipc__get_state(void) return IPC_STATE__OTHER_ERROR; } -int fsmonitor_ipc__send_query(const char *since_token, - struct strbuf *answer) +int fsmonitor_ipc__send_query(const char *since_token UNUSED, + struct strbuf *answer UNUSED) { return -1; } -int fsmonitor_ipc__send_command(const char *command, - struct strbuf *answer) +int fsmonitor_ipc__send_command(const char *command UNUSED, + struct strbuf *answer UNUSED) { return -1; } From 997eb910a644491b8793ae0459ee2c5d89f239f8 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 18 Sep 2023 18:32:56 -0400 Subject: [PATCH 6/8] fsmonitor/darwin: mark unused parameters in system callback We pass fsevent_callback() to the system FSEventStreamCreate() function as a callback. So we must match the expected function signature, even though we don't care about all of the parameters. Mark the unused ones to satisfy -Wunused-parameter. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- compat/fsmonitor/fsm-listen-darwin.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compat/fsmonitor/fsm-listen-darwin.c b/compat/fsmonitor/fsm-listen-darwin.c index 36c7e13281c675..11b56d3ef12ffb 100644 --- a/compat/fsmonitor/fsm-listen-darwin.c +++ b/compat/fsmonitor/fsm-listen-darwin.c @@ -191,12 +191,12 @@ static void my_add_path(struct fsmonitor_batch *batch, const char *path) } -static void fsevent_callback(ConstFSEventStreamRef streamRef, +static void fsevent_callback(ConstFSEventStreamRef streamRef UNUSED, void *ctx, size_t num_of_events, void *event_paths, const FSEventStreamEventFlags event_flags[], - const FSEventStreamEventId event_ids[]) + const FSEventStreamEventId event_ids[] UNUSED) { struct fsmonitor_daemon_state *state = ctx; struct fsm_listen_data *data = state->listen_data; From 1fe41944b268c5e999c5bbd9539e7dcd18c2c5de Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 18 Sep 2023 18:33:14 -0400 Subject: [PATCH 7/8] fsmonitor: mark unused hashmap callback parameters Like many hashmap comparison functions, our cookies_cmp() does not look at its extra void data parameter. This should have been annotated in 02c3c59e62 (hashmap: mark unused callback parameters, 2022-08-19), but this new case was added around the same time (plus fsmonitor is not built at all on Linux, so it is easy to miss there). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/fsmonitor--daemon.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index 7c4130c9811bd5..ce511c3ed6490b 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -129,8 +129,9 @@ struct fsmonitor_cookie_item { enum fsmonitor_cookie_item_result result; }; -static int cookies_cmp(const void *data, const struct hashmap_entry *he1, - const struct hashmap_entry *he2, const void *keydata) +static int cookies_cmp(const void *data UNUSED, + const struct hashmap_entry *he1, + const struct hashmap_entry *he2, const void *keydata) { const struct fsmonitor_cookie_item *a = container_of(he1, const struct fsmonitor_cookie_item, entry); From 72da9832c21c96b2109408d9de7b6ce45fff8847 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 18 Sep 2023 18:33:43 -0400 Subject: [PATCH 8/8] run-command: mark unused parameters in start_bg_wait callbacks The start_bg_command() function takes a callback to tell when the background-ed process is "ready". The callback receives the child_process struct as well as an extra void pointer. But curiously, neither of the two users of this interface look at either parameter! This makes some sense. The only non-test user of the API is fsmonitor, which uses fsmonitor_ipc__get_state() to connect to a single global fsmonitor daemon (i.e., the one we just started!). So we could just drop these parameters entirely. But it seems like a pretty reasonable interface for the "wait" callback to have access to the details of the spawned process, and to have room for passing extra data through a void pointer. So let's leave these in place but mark the unused ones so that -Wunused-parameter does not complain. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/fsmonitor--daemon.c | 3 ++- t/helper/test-simple-ipc.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index ce511c3ed6490b..5d01db5c029e2b 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -1443,7 +1443,8 @@ static int try_to_run_foreground_daemon(int detach_console MAYBE_UNUSED) static start_bg_wait_cb bg_wait_cb; -static int bg_wait_cb(const struct child_process *cp, void *cb_data) +static int bg_wait_cb(const struct child_process *cp UNUSED, + void *cb_data UNUSED) { enum ipc_active_state s = fsmonitor_ipc__get_state(); diff --git a/t/helper/test-simple-ipc.c b/t/helper/test-simple-ipc.c index 3d1436da59872f..941ae7e3bcf3ed 100644 --- a/t/helper/test-simple-ipc.c +++ b/t/helper/test-simple-ipc.c @@ -278,7 +278,8 @@ static int daemon__run_server(void) static start_bg_wait_cb bg_wait_cb; -static int bg_wait_cb(const struct child_process *cp, void *cb_data) +static int bg_wait_cb(const struct child_process *cp UNUSED, + void *cb_data UNUSED) { int s = ipc_get_active_state(cl_args.path);