[PATCH] Finish the task 'sysctl reporting current working directory'

[Mateusz Guzik]

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>