From owner-freebsd-hackers@FreeBSD.ORG Mon Nov 3 06:42:07 2014 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id AAD2014E for ; Mon, 3 Nov 2014 06:42:07 +0000 (UTC) Received: from ustc.edu.cn (email6.ustc.edu.cn [IPv6:2001:da8:d800::8]) by mx1.freebsd.org (Postfix) with ESMTP id 8ECD0F4B for ; Mon, 3 Nov 2014 06:42:05 +0000 (UTC) Received: from freebsd (unknown [58.211.218.74]) by newmailweb.ustc.edu.cn (Coremail) with SMTP id LkAmygBnbPSNI1dUuZTGAA--.1780S2; Mon, 03 Nov 2014 14:41:23 +0800 (CST) Date: Mon, 3 Nov 2014 14:40:52 +0800 From: Tiwei Bie To: Mateusz Guzik Subject: Re: [PATCH] Finish the task 'sysctl reporting current working directory' Message-ID: <20141103064052.GA1739@freebsd> References: <1414987325-23280-1-git-send-email-btw@mail.ustc.edu.cn> <20141103051908.GC29497@dft-labs.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20141103051908.GC29497@dft-labs.eu> User-Agent: Mutt/1.5.23 (2014-03-12) X-CM-TRANSID: LkAmygBnbPSNI1dUuZTGAA--.1780S2 X-Coremail-Antispam: 1UD129KBjvJXoW3Zry3XFW8tr4rZFW5Ar17Awb_yoWDWrWfpa n8AFsrtF47GF1UKr4vqr4ru3WSkw18XF1xWay8Ww43AFsYvr18Wr1Iqr909F1fu3y5u34S vF45KFn3Gw10yFJanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDU0xBIdaVrnRJUUUkjb7Iv0xC_Kw4lb4IE77IF4wAFF20E14v26r1j6r4UM7CY07I2 0VC2zVCF04k26cxKx2IYs7xG6rWj6s0DM7CIcVAFz4kK6r1j6r18M28lY4IEw2IIxxk0rw A2F7IY1VAKz4vEj48ve4kI8wA2z4x0Y4vE2Ix0cI8IcVAFwI0_Ar0_tr1l84ACjcxK6xII jxv20xvEc7CjxVAFwI0_Cr0_Gr1UM28EF7xvwVC2z280aVAFwI0_GcCE3s1l84ACjcxK6I 8E87Iv6xkF7I0E14v26rxl6s0DM2AIxVAIcxkEcVAq07x20xvEncxIr21l5I8CrVACY4xI 64kE6c02F40Ex7xfMcIj6xIIjxv20xvE14v26r1Y6r17McIj6I8E87Iv67AKxVW8JVWxJw Am72CE4IkC6x0Yz7v_Jr0_Gr1lF7xvr2IY64vIr41lc2xSY4AK67AK6r47MxAIw28IcxkI 7VAKI48JMxC20s026xCaFVCjc4AY6r1j6r4UMI8I3I0E5I8CrVAFwI0_Jr0_Jr4lx2IqxV Cjr7xvwVAFwI0_JrI_JrWlx4CE17CEb7AF67AKxVWUXVWUAwCIc40Y0x0EwIxGrwCI42IY 6xIIjxv20xvE14v26r1j6r1xMIIF0xvE2Ix0cI8IcVCY1x0267AKxVWUJVW8JwCI42IY6x AIw20EY4v20xvaj40_WFyUJVCq3wCI42IY6I8E87Iv67AKxVWUJVW8JwCI42IY6I8E87Iv 6xkF7I0E14v26r1j6r4UYxBIdaVFxhVjvjDU0xZFpf9x07b7XocUUUUU= X-CM-SenderInfo: xewzqzxdloh3xvwfhvlgxou0/1tbiAQURAVQhl8yZCAAEsv Cc: freebsd-hackers@freebsd.org X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Nov 2014 06:42:07 -0000 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