Re: [PATCH 2/4] add chdir-notify API
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Mar 28, 2018, 17:58 UTC
- Message-ID
- <CAPig+cQ0D+k+WmDWcaFHQj7DBb1RPfaUYwOvLosWXdHOzsoLOw@mail.gmail.com>
- In-Reply-To
- <20180328174041.GB16274@sigill.intra.peff.net>
On Wed, Mar 28, 2018 at 1:40 PM, Jeff King <peff@peff.net> wrote:
Show 20 quoted lines
> [...]
> Let's provide an API to let code that stores relative paths
> "subscribe" to updates to the current working directory.
> This means that callers of chdir() don't need to know about
> all subscribers ahead of time; they can simply consult a
> dynamically built list.
> [...]
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> diff --git a/chdir-notify.c b/chdir-notify.c
> @@ -0,0 +1,71 @@
> +int chdir_notify(const char *new_cwd)
> +{
> + struct strbuf old_cwd = STRBUF_INIT;
> + struct list_head *pos;
> +
> + if (strbuf_getcwd(&old_cwd) < 0)
> + return -1;
> + if (chdir(new_cwd) < 0)
> + return -1;This 'return' is leaking 'old_cwd', isn't it?
Show 22 quoted lines
> + list_for_each(pos, &chdir_notify_entries) {
> + struct chdir_notify_entry *e =
> + list_entry(pos, struct chdir_notify_entry, list);
> + e->cb(old_cwd.buf, new_cwd, e->data);
> + }
> +
> + strbuf_release(&old_cwd);
> + return 0;
> +}
> diff --git a/chdir-notify.h b/chdir-notify.h
> @@ -0,0 +1,64 @@
> + * In practice most callers will want to move a relative path to the new root;
> + * they can use the reparent_relative_path() helper for that. If that's all
> + * you're doing, you can also use the convenience function:
> + *
> + * chdir_notify_reparent(&my_path);
> + */
> +typedef void (*chdir_notify_callback)(const char *old_cwd,
> + const char *new_cwd,
> + void *data);
> +void chdir_notify_register(chdir_notify_callback cb, void *data);
> +void chdir_notify_reparent(char **path);Can we have some documentation here (or in the chdir_notify_reparent() example above) explaining that *path is a heap-allocated value? I had to consult the implementation to understand ownership.