This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

checking return values for pfiles.stp


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 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]