[PATCH] Make various minor cleanup fixes to operf

[PATCH] Make various minor cleanup fixes to operf

	- Remove or update stale comments
	- Add calls to cleanup() at error exit points
	- Add pre-check to verify user has access to sample data dir

Signed-off-by: Maynard Johnson <maynardj <at> us.ibm.com>
---
 libperf_events/operf_utils.cpp |    6 +----
 pe_profiling/operf.cpp         |   41 ++++++++++++++++++++++++++++++++++-----
 2 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/libperf_events/operf_utils.cpp b/libperf_events/operf_utils.cpp
index 1d18f32..4ff99c0 100644
--- a/libperf_events/operf_utils.cpp
+++ b/libperf_events/operf_utils.cpp
 <at>  <at>  -280,7 +280,6  <at>  <at>  static struct operf_transient * __get_operf_trans(struct sample_data * data)
 			proc = it->second;
 			trans.cur_procinfo = proc;
 		} else {
-			// TODO
 			/* This can happen for the following reasons:
 			 *   - We get a sample before getting a COMM or MMAP
 			 *     event for the process being profiled
 <at>  <at>  -291,10 +290,7  <at>  <at>  static struct operf_transient * __get_operf_trans(struct sample_data * data)
 			 *     process other than the one we requested (not
 			 *     likely -- this would be a kernel bug if it did)
 			 *
-			 * Need to look into caching these discarded samples and trying to
-			 * process them after we have a valid app name recorded.  This could
-			 * cause a lot of thrashing about.  But at the very least,
-			 * we need to log the lost sample.
+			 * TODO: log the lost sample if !first_time_processing
 			*/
 			if ((cverb << vmisc) && !first_time_processing)
 				cerr << "Dropping sample -- process info unavailable" << endl;
diff --git a/pe_profiling/operf.cpp b/pe_profiling/operf.cpp
index bab5b2a..4b2c65f 100644
--- a/pe_profiling/operf.cpp
+++ b/pe_profiling/operf.cpp
 <at>  <at>  -585,12 +585,13  <at>  <at>  static void complete(void)
 	}
 	if (operf_options::reset) {
 		int flags = FTW_DEPTH | FTW_ACTIONRETVAL;
-
+		errno = 0;
 		if (nftw(current_sampledir.c_str(), __delete_sample_data, 32, flags) !=0 &&
 				errno != ENOENT) {
-			cerr << "Problem encountered clearing old sample data."
-			     << " Possible permissions problem." << endl;
-			cerr << "Try a manual removal of " << current_sampledir << endl;
+			cerr << "Unable to remove old sample data at "
+			     << current_sampledir << "." << endl;
+			if (errno)
+				cerr << strerror(errno) << endl;
 			cleanup();
 			exit(1);
 		}
 <at>  <at>  -1066,8 +1067,6  <at>  <at>  static void process_args(int argc, char const ** argv)
 	_process_session_dir();
 	outputfile = samples_dir + "/" + DEFAULT_OPERF_OUTFILE;

-	// TODO: Need to examine ocontrol to see what (if any) additional
-	// event verification is needed here.
 	if (operf_options::evts.empty()) {
 		// Use default event
 		get_default_event();
 <at>  <at>  -1129,14 +1128,44  <at>  <at>  int main(int argc, char const *argv[])
 	my_uid = geteuid();
 	if (operf_options::system_wide && my_uid != 0) {
 		cerr << "You must be root to do system-wide profiling." << endl;
+		cleanup();
 		exit(1);
 	}

 	if (cpu_type == CPU_NO_GOOD) {
 		cerr << "Unable to ascertain cpu type.  Exiting." << endl;
+		cleanup();
 		exit(1);
 	}
 	op_nr_counters = op_get_nr_counters(cpu_type);
+
+	if (my_uid != 0) {
+		/* Pre-check to make sure we have permission to remove old sample data
+		 * or to create new sample data in the specified sample data directory.
+		 * If the user wants us to remove old data, we don't actually do it now,
+		 * since the profile session may fail for some reason or the user may do ctl-c.
+		 * We should exit without unnecessarily removing the old sample data as
+		 * the user may expect it to still be there after an aborted run.
+		 */
+		string current_sampledir = samples_dir + "/current";
+		string current_sampledir_testfile = current_sampledir + "/.xxxTeStFiLe";
+		ofstream afile;
+		errno = 0;
+		afile.open(current_sampledir_testfile.c_str());
+		if (!afile.is_open() && (errno != ENOENT)) {
+			if (operf_options::reset)
+				cerr << "Unable to remove old sample data at "
+				     << current_sampledir << "." << endl;
+			else
+				cerr << "Unable to write to sample data directory at "
+				     << current_sampledir << "." << endl;
+			if (errno)
+				cerr << strerror(errno) << endl;
+			cerr << "Try a manual removal of " << current_sampledir << endl;
+			cleanup();
+			exit(1);
+		}
+	}
 	end_code_t run_result;
 	if ((run_result = _run())) {
 		if (app_started && (run_result != APP_ABNORMAL_END)) {
--

-- 
1.6.2.rc2

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/