This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: checking return values for pfiles.stp
Updated with jistone's suggestions.
On Tue, Oct 19, 2010 at 11:49 AM, Erick Tryzelaar
<erick.tryzelaar@gmail.com> wrote:
> Good morning,
>
> jistone on #systemtap helped me clean up pfiles.stp to make sure to
> check return values when running pfiles.stp, which I've attached.
> Also, would there be any possibility to promote any of pfiles's
> functionality into a tapset? I find it pretty useful to be able to get
> a filename for a given fd.
>
> Thanks!
>
From f7a5a108b524bdceaca09473b6f7c1666e6c864f Mon Sep 17 00:00:00 2001
From: Erick Tryzelaar <erick.tryzelaar@gmail.com>
Date: Tue, 19 Oct 2010 17:04:00 -0700
Subject: [PATCH] Rewrite pfiles to check return values
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.7.2.1"
This is a multi-part message in MIME format.
--------------1.7.2.1
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit
---
testsuite/systemtap.examples/process/pfiles.stp | 239 ++++++++++++++---------
1 files changed, 145 insertions(+), 94 deletions(-)
--------------1.7.2.1
Content-Type: text/x-patch; name="0001-Rewrite-pfiles-to-check-return-values.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0001-Rewrite-pfiles-to-check-return-values.patch"
diff --git a/testsuite/systemtap.examples/process/pfiles.stp b/testsuite/systemtap.examples/process/pfiles.stp
index e0a923b..9b52fd3 100755
--- a/testsuite/systemtap.examples/process/pfiles.stp
+++ b/testsuite/systemtap.examples/process/pfiles.stp
@@ -85,20 +85,17 @@
function task_valid_file_handle:long (task:long, fd:long) %{ /* pure */
struct task_struct *p = (struct task_struct *)((long)THIS->task);
- struct files_struct *files = kread(&p->files);
+ struct files_struct *files;
struct file *filp;
rcu_read_lock();
- filp = fcheck_files(files, THIS->fd);
- if (!filp) {
- THIS->__retvalue = 0;
- rcu_read_unlock();
- return;
+ if ((files = kread(&p->files)) &&
+ (filp = fcheck_files(files, THIS->fd))) {
+ THIS->__retvalue = (long)filp;
}
- THIS->__retvalue = (long)filp;
- rcu_read_unlock();
CATCH_DEREF_FAULT();
+ rcu_read_unlock();
%}
function i_mode2str:string (i_mode:long) %{ /* pure */
@@ -116,203 +113,256 @@ function i_mode2str:string (i_mode:long) %{ /* pure */
function task_file_handle_i_mode:long (task:long, fd:long) %{ /* pure */
struct task_struct *p = (struct task_struct *)((long)THIS->task);
- struct files_struct *files = kread(&p->files);
+ struct files_struct *files;
struct file *filp;
+ struct dentry *dentry;
+ struct inode *inode;
rcu_read_lock();
- filp = fcheck_files(files, THIS->fd);
- THIS->__retvalue = kread(&filp->f_dentry->d_inode->i_mode);
- rcu_read_unlock();
+ if ((files = kread(&p->files)) &&
+ (filp = fcheck_files(files, THIS->fd)) &&
+ (dentry = kread(&filp->f_dentry)) &&
+ (inode = kread(&dentry->d_inode))) {
+ THIS->__retvalue = inode->i_mode;
+ }
CATCH_DEREF_FAULT();
+ rcu_read_unlock();
%}
function task_file_handle_majmin_dev:string (task:long, fd:long) %{ /* pure */
struct task_struct *p = (struct task_struct *)((long)THIS->task);
- struct files_struct *files = kread(&p->files);
+ struct files_struct *files;
struct file *filp;
+ struct dentry *dentry;
+ struct inode *inode;
+ struct super_block *sb;
int dev_nr;
rcu_read_lock();
- filp = fcheck_files(files, THIS->fd);
- dev_nr = kread(&filp->f_dentry->d_inode->i_sb->s_dev);
- snprintf(THIS->__retvalue, MAXSTRINGLEN,
- "%d,%d", MAJOR(dev_nr), MINOR(dev_nr));
- rcu_read_unlock();
+ if ((files = kread(&p->files)) &&
+ (filp = fcheck_files(files, THIS->fd)) &&
+ (dentry = kread(&filp->f_dentry)) &&
+ (inode = kread(&dentry->d_inode)) &&
+ (sb = kread(&inode->i_sb)) &&
+ (dev_nr = kread(&sb->s_dev))) {
+ snprintf(THIS->__retvalue, MAXSTRINGLEN,
+ "%d,%d", MAJOR(dev_nr), MINOR(dev_nr));
+ }
CATCH_DEREF_FAULT();
+ rcu_read_unlock();
%}
function task_file_handle_majmin_rdev:string (task:long, fd:long) %{ /* pure */
struct task_struct *p = (struct task_struct *)((long)THIS->task);
- struct files_struct *files = kread(&p->files);
+ struct files_struct *files;
struct file *filp;
+ struct dentry *dentry;
+ struct inode *inode;
int rdev_nr;
rcu_read_lock();
- filp = fcheck_files(files, THIS->fd);
- rdev_nr = kread(&filp->f_dentry->d_inode->i_rdev);
- snprintf(THIS->__retvalue, MAXSTRINGLEN,
- "%d,%d", MAJOR(rdev_nr), MINOR(rdev_nr));
- rcu_read_unlock();
+ if ((files = kread(&p->files)) &&
+ (filp = fcheck_files(files, THIS->fd)) &&
+ (dentry = kread(&filp->f_dentry)) &&
+ (inode = kread(&dentry->d_inode)) &&
+ (rdev_nr = kread(&inode->i_rdev))) {
+ snprintf(THIS->__retvalue, MAXSTRINGLEN,
+ "%d,%d", MAJOR(rdev_nr), MINOR(rdev_nr));
+ }
CATCH_DEREF_FAULT();
+ rcu_read_unlock();
%}
function task_file_handle_i_node:long (task:long, fd:long) %{ /* pure */
struct task_struct *p = (struct task_struct *)((long)THIS->task);
- struct files_struct *files = kread(&p->files);
+ struct files_struct *files;
struct file *filp;
+ struct dentry *dentry;
+ struct inode *inode;
rcu_read_lock();
- filp = fcheck_files(files, THIS->fd);
- THIS->__retvalue = kread(&filp->f_dentry->d_inode->i_ino);
- rcu_read_unlock();
+ if ((files = kread(&p->files)) &&
+ (filp = fcheck_files(files, THIS->fd)) &&
+ (dentry = kread(&filp->f_dentry)) &&
+ (inode = kread(&dentry->d_inode))) {
+ THIS->__retvalue = inode->i_ino;
+ }
CATCH_DEREF_FAULT();
+ rcu_read_unlock();
%}
function task_file_handle_uid:long (task:long, fd:long) %{ /* pure */
struct task_struct *p = (struct task_struct *)((long)THIS->task);
- struct files_struct *files = kread(&p->files);
+ struct files_struct *files;
struct file *filp;
rcu_read_lock();
- filp = fcheck_files(files, THIS->fd);
+ if ((files = kread(&p->files)) &&
+ (filp = fcheck_files(files, THIS->fd))) {
#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,29)
- /* git commit d76b0d9b */
- THIS->__retvalue = kread(&filp->f_cred->fsuid);
+ const struct cred *cred;
+ if ((cred = kread(&filp->f_cred))) {
+ /* git commit d76b0d9b */
+ THIS->__retvalue = cred->fsuid;
+ }
#else
- THIS->__retvalue = kread(&filp->f_uid);
+ THIS->__retvalue = kread(&filp->f_uid);
#endif
- rcu_read_unlock();
+ }
CATCH_DEREF_FAULT();
+ rcu_read_unlock();
%}
function task_file_handle_gid:long (task:long, fd:long) %{ /* pure */
struct task_struct *p = (struct task_struct *)((long)THIS->task);
- struct files_struct *files = kread(&p->files);
+ struct files_struct *files;
struct file *filp;
rcu_read_lock();
- filp = fcheck_files(files, THIS->fd);
+ if ((files = kread(&p->files)) &&
+ (filp = fcheck_files(files, THIS->fd))) {
#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,29)
- /* git commit d76b0d9b */
- THIS->__retvalue = kread(&filp->f_cred->fsgid);
+ const struct cred *cred;
+ if ((cred = kread(&filp->f_cred))) {
+ /* git commit d76b0d9b */
+ THIS->__retvalue = cred->fsgid;
+ }
#else
- THIS->__retvalue = kread(&filp->f_gid);
+ THIS->__retvalue = kread(&filp->f_gid);
#endif
- rcu_read_unlock();
+ }
CATCH_DEREF_FAULT();
+ rcu_read_unlock();
%}
function task_file_handle_f_flags:long (task:long, fd:long) %{ /* pure */
struct task_struct *p = (struct task_struct *)((long)THIS->task);
- struct files_struct *files = kread(&p->files);
+ struct files_struct *files;
struct file *filp;
rcu_read_lock();
- filp = fcheck_files(files, THIS->fd);
- THIS->__retvalue = kread(&filp->f_flags);
- rcu_read_unlock();
+ if ((files = kread(&p->files)) &&
+ (filp = fcheck_files(files, THIS->fd))) {
+ THIS->__retvalue = kread(&filp->f_flags);
+ }
CATCH_DEREF_FAULT();
+ rcu_read_unlock();
%}
function task_file_handle_fd_flags:string (task:long, fd:long) %{ /* pure */
struct task_struct *p = (struct task_struct *)((long)THIS->task);
- struct files_struct *files = kread(&p->files);
+ struct files_struct *files;
struct fdtable *fdt;
int gcoe;
rcu_read_lock();
- fdt = files_fdtable(files);
- gcoe = FD_ISSET(THIS->fd, kread(&fdt->close_on_exec));
- snprintf(THIS->__retvalue, MAXSTRINGLEN,
- "%s", gcoe ? "FD_CLOEXEC" : "");
- rcu_read_unlock();
+ if ((files = kread(&p->files)) &&
+ (fdt = files_fdtable(files))) {
+ gcoe = FD_ISSET(THIS->fd, kread(&fdt->close_on_exec));
+ snprintf(THIS->__retvalue, MAXSTRINGLEN,
+ "%s", gcoe ? "FD_CLOEXEC" : "");
+ }
CATCH_DEREF_FAULT();
+ rcu_read_unlock();
%}
function task_file_handle_flock:string (task:long, fd:long) %{ /* pure */
struct task_struct *p = (struct task_struct *)((long)THIS->task);
- struct files_struct *files = kread(&p->files);
+ struct files_struct *files;
struct file_lock *flock;
int fl_type, fl_pid;
struct file *filp;
+ struct dentry *dentry;
+ struct inode *inode;
rcu_read_lock();
- filp = fcheck_files(files, THIS->fd);
- flock = kread(&filp->f_dentry->d_inode->i_flock);
- fl_type = fl_pid = -1;
- if (flock) {
- fl_type = kread(&flock->fl_type);
- fl_pid = kread(&flock->fl_pid);
- }
+ if ((files = kread(&p->files)) &&
+ (filp = fcheck_files(files, THIS->fd)) &&
+ (dentry = kread(&filp->f_dentry)) &&
+ (inode = kread(&dentry->d_inode))) {
+ if ((flock = kread(&inode->i_flock))) {
+ fl_type = kread(&flock->fl_type);
+ fl_pid = kread(&flock->fl_pid);
+ } else {
+ fl_type = -1;
+ fl_pid = -1;
+ }
- if (fl_type != -1) /* !flock */
- snprintf(THIS->__retvalue, MAXSTRINGLEN,
- " advisory %s lock set by process %d",
- fl_type ? "write" : "read", fl_pid);
- else
- snprintf(THIS->__retvalue, MAXSTRINGLEN, "NULL");
- rcu_read_unlock();
+ if (fl_type != -1) { /* !flock */
+ snprintf(THIS->__retvalue, MAXSTRINGLEN,
+ " advisory %s lock set by process %d",
+ fl_type ? "write" : "read", fl_pid);
+ } else {
+ snprintf(THIS->__retvalue, MAXSTRINGLEN, "NULL");
+ }
+ }
CATCH_DEREF_FAULT();
+ rcu_read_unlock();
%}
function task_file_handle_d_path:string (task:long, fd:long) %{ /* pure */
struct task_struct *p = (struct task_struct *)((long)THIS->task);
- struct files_struct *files = kread(&p->files);
- char *page = (char *)__get_free_page(GFP_KERNEL);
+ struct files_struct *files;
+ char *page = NULL;
struct file *filp;
struct dentry *dentry;
struct vfsmount *vfsmnt;
-
- /* TODO: handle !page */
- if (!page)
- return;
+ char *path = NULL;
rcu_read_lock();
- filp = fcheck_files(files, THIS->fd);
- dentry = kread(&filp->f_dentry);
- vfsmnt = kread(&filp->f_vfsmnt);
+ if ((files = kread(&p->files)) &&
+ (page = (char *)__get_free_page(GFP_KERNEL)) &&
+ (filp = fcheck_files(files, THIS->fd))) {
+
#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,26)
- /* git commit 9d1bc601 */
- snprintf(THIS->__retvalue, MAXSTRINGLEN, "%s",
- d_path(&filp->f_path, page, PAGE_SIZE));
+ /* git commit 9d1bc601 */
+ path = d_path(&filp->f_path, page, PAGE_SIZE);
#else
- snprintf(THIS->__retvalue, MAXSTRINGLEN, "%s",
- d_path(dentry, vfsmnt, page, PAGE_SIZE));
-#endif
- free_page((unsigned long)page);
- rcu_read_unlock();
+ dentry = kread(&filp->f_dentry);
+ vfsmnt = kread(&filp->f_vfsmnt);
+ if (dentry && vfsmnt) {
+ path = d_path(dentry, vfsmnt, page, PAGE_SIZE);
+ }
+#endif
+ if (path && !IS_ERR(path)) {
+ snprintf(THIS->__retvalue, MAXSTRINGLEN, "%s", path);
+ }
+ }
CATCH_DEREF_FAULT();
+
+ if (page) free_page((unsigned long)page);
+
+ rcu_read_unlock();
%}
function task_file_handle_socket:long (task:long, fd:long) %{ /* pure */
struct task_struct *p = (struct task_struct *)((long)THIS->task);
- struct files_struct *files = kread(&p->files);
- struct inode *inode;
+ struct files_struct *files;
struct file *filp;
+ struct dentry *dentry;
+ struct inode *inode;
rcu_read_lock();
- filp = fcheck_files(files, THIS->fd);
- if (!filp) {
- THIS->__retvalue = 0;
- rcu_read_unlock();
- return;
+ if ((files = kread(&p->files)) &&
+ (filp = fcheck_files(files, THIS->fd)) &&
+ (dentry = kread(&filp->f_dentry)) &&
+ (inode = kread(&filp->f_dentry->d_inode))) {
+ if (S_ISSOCK(kread(&inode->i_mode)))
+ THIS->__retvalue = (long)SOCKET_I(inode);
}
- inode = kread(&filp->f_dentry->d_inode);
- if (S_ISSOCK(kread(&inode->i_mode)))
- THIS->__retvalue = (long)SOCKET_I(inode);
- rcu_read_unlock();
CATCH_DEREF_FAULT();
+ rcu_read_unlock();
%}
function socket_userlocks:string (sock:long) %{ /* pure */
@@ -391,7 +441,8 @@ function socket_optname:string (sock:long) %{ /* pure */
function socket_family:long (sock:long) %{ /* pure */
struct socket *sock = (struct socket *)((long)THIS->sock);
- THIS->__retvalue = (long)kread(&sock->ops->family);
+ const struct proto_ops *ops = kread(&sock->ops);
+ THIS->__retvalue = (long)kread(ops->family);
CATCH_DEREF_FAULT();
%}
--------------1.7.2.1--