On Wed, Mar 28, 2018 at 1:40 PM, Jeff King <[email protected]> wrote:
> [...]
> 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 <[email protected]>
> ---
> 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?
> + 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.
