Hi Jeff,
On Fri, 11 Feb 2022, Jeff Hostetler via GitGitGadget wrote:
> From: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
>
> Create fsmonitor_ipc__*() client routines to spawn the built-in file
> system monitor daemon and send it an IPC request using the `Simple
> IPC` API.
>
> Stub in empty fsmonitor_ipc__*() functions for unsupported platforms.
This looks very straight-forward, thanks to your design of the Simple IPC.
Just one thing:
> diff --git a/fsmonitor-ipc.c b/fsmonitor-ipc.c
> new file mode 100644
> index 00000000000..01c8c25bf50
> --- /dev/null
> +++ b/fsmonitor-ipc.c
> @@ -0,0 +1,171 @@
> +#include "cache.h"
> +#include "fsmonitor.h"
> +#include "simple-ipc.h"
> +#include "fsmonitor-ipc.h"
> +#include "run-command.h"
> +#include "strbuf.h"
> +#include "trace2.h"
> +
> +#ifdef HAVE_FSMONITOR_DAEMON_BACKEND
> +
> [...]
> +
> +#else
> +
> +/*
> + * A trivial implementation of the fsmonitor_ipc__ API for unsupported
> + * platforms.
> + */
> +
> +int fsmonitor_ipc__is_supported(void)
> +{
> + return 0;
> +}
> +
> +const char *fsmonitor_ipc__get_path(void)
> +{
> + return NULL;
> +}
> +
> +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)
> +{
> + return -1;
> +}
> +
> +int fsmonitor_ipc__send_command(const char *command,
> + struct strbuf *answer)
> +{
> + return -1;
> +}
> +
> +#endif
Not a big deal (read: I won't mind if you leave the code as-is), but I
usually find it easier to read the shorter, more trivial arm of
conditional if/else arms first. In this instance, the dummy implementation
for platforms that are not (yet) supported looks more trivial to me.
Thanks,
Dscho
