Date: Mon, 3 Nov 2014 06:19:08 +0100 From: Mateusz Guzik <mjguzik@gmail.com> To: Tiwei Bie <btw@mail.ustc.edu.cn> Cc: freebsd-hackers@freebsd.org, mjg@freebsd.org Subject: Re: [PATCH] Finish the task 'sysctl reporting current working directory' Message-ID: <20141103051908.GC29497@dft-labs.eu> In-Reply-To: <1414987325-23280-1-git-send-email-btw@mail.ustc.edu.cn> References: <1414987325-23280-1-git-send-email-btw@mail.ustc.edu.cn>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Nov 03, 2014 at 12:02:05PM +0800, Tiwei Bie wrote:
> Hi, Mateusz!
>
> I have finished the task: sysctl reporting current working directory [1].
>
> The patch for tmux is against tmux-1.9a [2].
>
> #1. Patch for FreeBSD:
>
> diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
> index 11ab4ba..4652cb9 100644
> --- a/sys/kern/kern_descrip.c
> +++ b/sys/kern/kern_descrip.c
> @@ -3409,6 +3409,72 @@ static SYSCTL_NODE(_kern_proc, KERN_PROC_FILEDESC, filedesc,
> CTLFLAG_RD|CTLFLAG_MPSAFE, sysctl_kern_proc_filedesc,
> "Process filedesc entries");
>
> +/*
> + * Store a process current working directory information to sbuf.
> + *
> + * Takes a locked proc as argument, and returns with the proc unlocked.
> + */
> +int
> +kern_proc_cwd_out(struct proc *p, struct sbuf *sb, ssize_t maxlen)
> +{
> + struct filedesc *fdp;
> + struct export_fd_buf *efbuf;
> + int error = 0;
> +
> + PROC_LOCK_ASSERT(p, MA_OWNED);
> +
> + fdp = fdhold(p);
> + PROC_UNLOCK(p);
> +
> + if (fdp == NULL)
> + return (error);
> +
This returns 0 which does not seem to be the righit choice.
EINVAL seems more suitable.
> + efbuf = malloc(sizeof(*efbuf), M_TEMP, M_WAITOK);
> + efbuf->fdp = fdp;
> + efbuf->sb = sb;
> + efbuf->remainder = maxlen;
> +
> + FILEDESC_SLOCK(fdp);
> + /* working directory */
Unnecessary comment.
> + if (fdp->fd_cdir != NULL) {
> + vref(fdp->fd_cdir);
> + export_vnode_to_sb(fdp->fd_cdir, KF_FD_TYPE_CWD, FREAD, efbuf);
Missing error checking.
> + }
> + FILEDESC_SUNLOCK(fdp);
> + fddrop(fdp);
> + free(efbuf, M_TEMP);
> + return (error);
So as it is the function always returns 0.
Other than that kernel part seems fine.
> +}
> +
> +/*
> + * Get per-process current working directory.
> + */
> +static int
> +sysctl_kern_proc_cwd(SYSCTL_HANDLER_ARGS)
> +{
> + struct sbuf sb;
> + struct proc *p;
> + ssize_t maxlen;
> + int error, error2, *name;
> +
> + name = (int *)arg1;
> +
> + sbuf_new_for_sysctl(&sb, NULL, sizeof(struct kinfo_file), req);
> + error = pget((pid_t)name[0], PGET_CANDEBUG | PGET_NOTWEXIT, &p);
> + if (error != 0) {
> + sbuf_delete(&sb);
> + return (error);
> + }
> + maxlen = req->oldptr != NULL ? req->oldlen : -1;
> + error = kern_proc_cwd_out(p, &sb, maxlen);
> + error2 = sbuf_finish(&sb);
> + sbuf_delete(&sb);
> + return (error != 0 ? error : error2);
> +}
> +
> +static SYSCTL_NODE(_kern_proc, KERN_PROC_CWD, cwd, CTLFLAG_RD|CTLFLAG_MPSAFE,
> + sysctl_kern_proc_cwd, "Process current working directory");
> +
> #ifdef DDB
> /*
> * For the purposes of debugging, generate a human-readable string for the
> diff --git a/sys/sys/sysctl.h b/sys/sys/sysctl.h
> index d782375..f3173c2 100644
> --- a/sys/sys/sysctl.h
> +++ b/sys/sys/sysctl.h
> @@ -657,6 +657,7 @@ TAILQ_HEAD(sysctl_ctx_list, sysctl_ctx_entry);
> #define KERN_PROC_UMASK 39 /* process umask */
> #define KERN_PROC_OSREL 40 /* osreldate for process binary */
> #define KERN_PROC_SIGTRAMP 41 /* signal trampoline location */
> +#define KERN_PROC_CWD 42 /* process current working directory */
>
> /*
> * KERN_IPC identifiers
> diff --git a/sys/sys/user.h b/sys/sys/user.h
> index f0d059e..c3b3bc5 100644
> --- a/sys/sys/user.h
> +++ b/sys/sys/user.h
> @@ -530,6 +530,7 @@ struct sbuf;
> */
>
> int kern_proc_filedesc_out(struct proc *p, struct sbuf *sb, ssize_t maxlen);
> +int kern_proc_cwd_out(struct proc *p, struct sbuf *sb, ssize_t maxlen);
> int kern_proc_out(struct proc *p, struct sbuf *sb, int flags);
> int kern_proc_vmmap_out(struct proc *p, struct sbuf *sb);
>
> --
> 2.1.0
>
>
> #2. Patch for tmux:
>
> diff --git a/osdep-freebsd.c b/osdep-freebsd.c
> index d596eab..f2443b7 100644
> --- a/osdep-freebsd.c
> +++ b/osdep-freebsd.c
> @@ -136,26 +136,39 @@ char *
> osdep_get_cwd(int fd)
> {
> static char wd[PATH_MAX];
> - struct kinfo_file *info = NULL;
> pid_t pgrp;
> - int nrecords, i;
> + int mib[4];
> + size_t len;
> + struct kinfo_file *info;
> + char *buf;
> + int error;
>
> if ((pgrp = tcgetpgrp(fd)) == -1)
> return (NULL);
>
> - if ((info = kinfo_getfile(pgrp, &nrecords)) == NULL)
> + mib[0] = CTL_KERN;
> + mib[1] = KERN_PROC;
> + mib[2] = KERN_PROC_CWD;
> + mib[3] = pgrp;
> +
> + error = sysctl(mib, 4, NULL, &len, NULL, 0);
> + if (error)
> return (NULL);
>
> - for (i = 0; i < nrecords; i++) {
> - if (info[i].kf_fd == KF_FD_TYPE_CWD) {
> - strlcpy(wd, info[i].kf_path, sizeof wd);
> - free(info);
> - return (wd);
> - }
> + buf = malloc(len);
> + if (buf == NULL)
> + return (NULL);
> + error = sysctl(mib, 4, buf, &len, NULL, 0);
> + if (error) {
> + free(buf);
> + return (NULL);
> }
>
> - free(info);
> - return (NULL);
> + info = (struct kinfo_file *)buf;
> + strlcpy(wd, info->kf_path, sizeof wd);
> +
> + free(buf);
> + return (wd);
This cannot be right. tmux has to remain compilable on versions without
this new sysctl.
You can ifdef the code based on KERN_PROC_CWD.
> }
>
> struct event_base *
> --
> 2.1.0
>
> [1] https://wiki.freebsd.org/JuniorJobs#sysctl_reporting_current_working_directory
> [2] http://cznic.dl.sourceforge.net/project/tmux/tmux/tmux-1.9/tmux-1.9a.tar.gz
>
> Tiwei Bie
>
--
Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141103051908.GC29497>