public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
* checking return values for pfiles.stp
@ 2010-10-19 18:49 Erick Tryzelaar
  2010-10-20  0:09 ` Erick Tryzelaar
  0 siblings, 1 reply; 3+ messages in thread
From: Erick Tryzelaar @ 2010-10-19 18:49 UTC (permalink / raw)
  To: systemtap

[-- Attachment #1: Type: text/plain, Size: 320 bytes --]

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!

[-- Attachment #2: 0001-Rewrite-pfiles-to-check-return-values.patch --]
[-- Type: text/x-patch, Size: 10829 bytes --]

From f9af2b7b9cf4d9e58581d8712da39280bcf6f7f5 Mon Sep 17 00:00:00 2001
From: Erick Tryzelaar <erick.tryzelaar@gmail.com>
Date: Tue, 19 Oct 2010 11:40:17 -0700
Subject: [PATCH] Rewrite pfiles to check return values

---
 testsuite/systemtap.examples/process/pfiles.stp |  218 +++++++++++++----------
 1 files changed, 126 insertions(+), 92 deletions(-)

diff --git a/testsuite/systemtap.examples/process/pfiles.stp b/testsuite/systemtap.examples/process/pfiles.stp
index e0a923b..3dd1dc7 100755
--- a/testsuite/systemtap.examples/process/pfiles.stp
+++ b/testsuite/systemtap.examples/process/pfiles.stp
@@ -85,20 +85,18 @@
 
 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))) {
+		if ((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 +114,239 @@ 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;
 
 	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))) {
+		if ((filp = fcheck_files(files, THIS->fd))) {
+			THIS->__retvalue = kread(&filp->f_dentry->d_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;
 	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))) {
+		if ((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));
+		}
+	}
 
 	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;
 	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))) {
+		if ((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));
+		}
+	}
 
 	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;
 	
 	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))) {
+		if ((filp = fcheck_files(files, THIS->fd))) {
+			THIS->__retvalue = kread(&filp->f_dentry->d_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))) {
+		if ((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);
+			/* git commit d76b0d9b */
+			THIS->__retvalue = kread(&filp->f_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))) {
+		if ((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);
+			/* git commit d76b0d9b */
+			THIS->__retvalue = kread(&filp->f_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))) {
+		if ((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))) {
+		if ((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;
 
 	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))) {
+		if ((filp = fcheck_files(files, THIS->fd))) {
+			if ((flock = kread(&filp->f_dentry->d_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))) {
+		/* TODO: handle !page */
+		if ((page = (char *)__get_free_page(GFP_KERNEL))) {
+			if ((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 files_struct *files;
 	struct inode *inode;
 	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))) {
+		if ((filp = fcheck_files(files, THIS->fd))) {
+			inode = kread(&filp->f_dentry->d_inode);
+			if (inode && 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 */
-- 
1.7.2.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-10-21  2:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-19 18:49 checking return values for pfiles.stp Erick Tryzelaar
2010-10-20  0:09 ` Erick Tryzelaar
2010-10-21  2:53   ` Josh Stone

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).