Re: [PATCH 2/4] add chdir-notify API

> On Wed, Mar 28, 2018 at 1:40 PM, Jeff King <peff@peff.net> 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 <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?

> > 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.