On Wed, Mar 28, 2018 at 7:40 PM, Jeff King <[email protected]> wrote:
> +static void reparent_cb(const char *old_cwd,
> + const char *new_cwd,
> + void *data)
> +{
> + char **path = data;
Maybe check data == NULL and return early. This is just for
convenience, e.g. instead of doing
path = getenv("foo");
if (path)
chdir_notify_reparent(&path);
I could do
path = getenv("foo");
chdir_notify_reparent(&path);
> + char *tmp = *path;
> + *path = reparent_relative_path(old_cwd, new_cwd, tmp);
> + free(tmp);
> +}
> +
> +void chdir_notify_reparent(char **path)
> +{
> + if (!is_absolute_path(*path))
I think this check is a bit early. There could be a big gap between
chdir_notify_reparent() and reparent_cb(). During that time, *path may
be updated and become a relative path. We can check for absolute path
inside reparent_cb() instead.
> + chdir_notify_register(reparent_cb, path);
> +}
Overall, I like this API very much! I will add another one similar to
chdir_notify_reparent() but it takes an env name instead and the
callback will setenv() appropriately. The result looks super good.
--
Duy
