Date: Mon, 3 Nov 2014 14:40:52 +0800 From: Tiwei Bie <btw@mail.ustc.edu.cn> To: Mateusz Guzik <mjguzik@gmail.com> Cc: freebsd-hackers@freebsd.org Subject: Re: [PATCH] Finish the task 'sysctl reporting current working directory' Message-ID: <20141103064052.GA1739@freebsd> In-Reply-To: <20141103051908.GC29497@dft-labs.eu> References: <1414987325-23280-1-git-send-email-btw@mail.ustc.edu.cn> <20141103051908.GC29497@dft-labs.eu>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Nov 03, 2014 at 06:19:08AM +0100, Mateusz Guzik wrote:
> 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.
>
I got it, this is a different situation with kern_proc_filedesc_out(),
In kern_proc_filedesc_out(), data about tracevp, textvp and cttyvp has
already been got. So it just returns 0. I mixed them. Thank you.
int
kern_proc_filedesc_out(struct proc *p, struct sbuf *sb, ssize_t maxlen)
{
......
error = 0;
if (fdp == NULL)
goto fail;
......
fail:
free(efbuf, M_TEMP);
return (error);
}
> > + 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.
>
I have considered checking its return value like this:
if (fdp->fd_cdir != NULL) {
vref(fdp->fd_cdir);
error = export_vnode_to_sb(fdp->fd_cdir, KF_FD_TYPE_CWD,
FREAD, efbuf);
}
But the code in kern_proc_filedesc_out() doesn't do this. So, I don't
know whether I should return this error code.
But now, as we have only to deal with current working directory, and
no other data will be exported, I think we could return with this error
code.
> > + }
> > + 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.
>
I got it, thank you!
Following are my new patches:
#1. Patch for FreeBSD:
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 2d1a5af..949c2d3 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -3410,6 +3410,75 @@ 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;
+
+ PROC_LOCK_ASSERT(p, MA_OWNED);
+
+ fdp = fdhold(p);
+ PROC_UNLOCK(p);
+
+ if (fdp == NULL)
+ return (EINVAL);
+
+ efbuf = malloc(sizeof(*efbuf), M_TEMP, M_WAITOK);
+ efbuf->fdp = fdp;
+ efbuf->sb = sb;
+ efbuf->remainder = maxlen;
+
+ FILEDESC_SLOCK(fdp);
+ if (fdp->fd_cdir == NULL)
+ error = EINVAL;
+ else {
+ vref(fdp->fd_cdir);
+ error = export_vnode_to_sb(fdp->fd_cdir, KF_FD_TYPE_CWD,
+ FREAD, efbuf);
+ }
+
+ FILEDESC_SUNLOCK(fdp);
+ fddrop(fdp);
+ free(efbuf, M_TEMP);
+ return (error);
+}
+
+/*
+ * 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..4178f01 100644
--- a/osdep-freebsd.c
+++ b/osdep-freebsd.c
@@ -132,6 +132,46 @@ error:
return (NULL);
}
+#ifdef KERN_PROC_CWD
+char *
+osdep_get_cwd(int fd)
+{
+ static char wd[PATH_MAX];
+ pid_t pgrp;
+ int mib[4];
+ size_t len;
+ struct kinfo_file *info;
+ char *buf;
+ int error;
+
+ if ((pgrp = tcgetpgrp(fd)) == -1)
+ return (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);
+
+ buf = malloc(len);
+ if (buf == NULL)
+ return (NULL);
+ error = sysctl(mib, 4, buf, &len, NULL, 0);
+ if (error) {
+ free(buf);
+ return (NULL);
+ }
+
+ info = (struct kinfo_file *)buf;
+ strlcpy(wd, info->kf_path, sizeof wd);
+
+ free(buf);
+ return (wd);
+}
+#else /* !KERN_PROC_CWD */
char *
osdep_get_cwd(int fd)
{
@@ -157,6 +197,7 @@ osdep_get_cwd(int fd)
free(info);
return (NULL);
}
+#endif /* KERN_PROC_CWD */
struct event_base *
osdep_event_init(void)
--
2.1.0
Tiwei Bie
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141103064052.GA1739>