On Tue, Sep 27 2022, Eric DeCosta via GitGitGadget wrote:
> From: Eric DeCosta <edecosta@xxxxxxxxxxxxx>
>
> + if (fsmonitor__get_alias(state.path_worktree_watch.buf, &state.alias)) {
> + err = error(_("could not get worktree alias"));
We could keep this, but if we'd error() in the function itself we'd just
emit two error() lines when one would do.
> + dir = opendir("/");
We should have a:
const char *const dir = "/";
So we can pass it to this..
> + if (!dir) {
> + error_errno("opendir('/') failed");
...and as a parameter to a _()-translated string. That way the next
translation doesn't need to translate opendir('/home') or whatever.
> + return -1;
Just skip the braces and do "return error_errno()", these functions return -1 for thath reason.
> + strbuf_init(&alias, 256);
>
> + /* no way of knowing what the link will resolve to, so MAXPATHLEN */
> + strbuf_init(&points_to, MAXPATHLEN);
Still need manual memory juggling? Ok.
> +
> + while ((de = readdir(dir)) != NULL) {
> + strbuf_reset(&alias);
> + strbuf_addch(&alias, '/');
> + strbuf_add(&alias, de->d_name, strlen(de->d_name));
strbuf_addf(&alias, "/%s", de->d_name);
Or rather:
strbuf_addf(&alias, "%s%s", root_dir, de->d_name);
If you split that "/" into a variable?
> +
> + read = readlink(alias.buf, points_to.buf, MAXPATHLEN);
> + if (read > 0) {
> + strbuf_setlen(&points_to, read);
> + if ((!strncmp(points_to.buf, path, points_to.len))
> + && path[points_to.len] == '/') {
> + strbuf_addbuf(&info->alias, &alias);
> + strbuf_addbuf(&info->points_to, &points_to);
> + trace_printf_key(&trace_fsmonitor,
> + "Found alias for '%s' : '%s' -> '%s'",
> + path, info->alias.buf, info->points_to.buf);
> + retval = 0;
> + goto done;
> + }
> + } else if (!read) {
> + BUG("readlink returned 0");
> + } else if (errno != EINVAL) { /* Something other than not a link */
> + error_errno("readlink('%s') failed", alias.buf);
> + goto done;
> + }
> + }
> + retval = 0; /* no alias */
> +
> +done:
> + if (closedir(dir) < 0)
> + warning_errno("closedir('/') failed");
Why not return an error for this? If you can't close the dir that
usually means the write or similar didn't work.
