On Wed, Mar 28, 2018 at 01:58:18PM -0400, Eric Sunshine wrote:

On Wed, Mar 28, 2018 at 1:40 PM, Jeff King [off-list ref] wrote:

quoted

[...]
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 <redacted>
---

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?

Whoops, yes. I wrote it the other way around first to avoid the leak,
but of course that has other problems. ;)

quoted

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.

Sure. I originally had reparent_relative_path() handle the freeing and
allocation, and it explained the ownership in its docstring. But as I
revised that to simply return its value, that got lost. I'll add it back
in here.

-Peff